[meta-freescale] [PATCH 0/1] arm: imx: fsl_otp: make fuses (OTP memory) read-only

Nothing in userspace should be able to kill the HW.
Not even just as root and for sure not that easy.

For obvious reason, I haven't tested this patch thoroughly.

Reported-by: Jon Nettleton <***@gmail.com>
Signed-off-by: Alexander Holler <***@ahsoftware.de>
---
drivers/char/fsl_otp.c | 57 +-------------------------------------------------
1 file changed, 1 insertion(+), 56 deletions(-)

diff --git a/drivers/char/fsl_otp.c b/drivers/char/fsl_otp.c
index face297..d9d1c59 100644
--- a/drivers/char/fsl_otp.c
+++ b/drivers/char/fsl_otp.c
@@ -151,60 +151,6 @@ out:
return ret ? 0 : sprintf(buf, "0x%x\n", value);
}

-static int otp_write_bits(int addr, u32 data, u32 magic)
-{
- u32 c; /* for control register */
-
- /* init the control register */
- c = __raw_readl(otp_base + HW_OCOTP_CTRL);
- c &= ~BM_OCOTP_CTRL_ADDR;
- c |= BF(addr, OCOTP_CTRL_ADDR);
- c |= BF(magic, OCOTP_CTRL_WR_UNLOCK);
- __raw_writel(c, otp_base + HW_OCOTP_CTRL);
-
- /* init the data register */
- __raw_writel(data, otp_base + HW_OCOTP_DATA);
- otp_wait_busy(0);
-
- mdelay(2); /* Write Postamble */
-
- return 0;
-}
-
-static ssize_t fsl_otp_store(struct kobject *kobj, struct kobj_attribute *attr,
- const char *buf, size_t count)
-{
- unsigned int index = attr - otp_kattr;
- u32 value;
- int ret;
-
- sscanf(buf, "0x%x", &value);
-
- ret = clk_prepare_enable(otp_clk);
- if (ret)
- return 0;
-
- mutex_lock(&otp_mutex);
-
- set_otp_timing();
- ret = otp_wait_busy(0);
- if (ret)
- goto out;
-
- otp_write_bits(index, value, 0x3e77);
-
- /* Reload all the shadow registers */
- __raw_writel(BM_OCOTP_CTRL_RELOAD_SHADOWS,
- otp_base + HW_OCOTP_CTRL_SET);
- udelay(1);
- otp_wait_busy(BM_OCOTP_CTRL_RELOAD_SHADOWS);
-
-out:
- mutex_unlock(&otp_mutex);
- clk_disable_unprepare(otp_clk);
- return ret ? 0 : count;
-}
-
static int fsl_otp_probe(struct platform_device *pdev)
{
struct resource *res;
@@ -244,9 +190,8 @@ static int fsl_otp_probe(struct platform_device *pdev)
for (i = 0; i < num; i++) {
sysfs_attr_init(&otp_kattr[i].attr);
otp_kattr[i].attr.name = desc[i];
- otp_kattr[i].attr.mode = 0600;
+ otp_kattr[i].attr.mode = 0400;
otp_kattr[i].show = fsl_otp_show;
- otp_kattr[i].store = fsl_otp_store;
attrs[i] = &otp_kattr[i].attr;
}
otp_attr_group->attrs = attrs;

--
1.8.3.1

--

Otavio Salvador

2014-11-07 11:34:11 UTC

Post by Alexander Holler
Nothing in userspace should be able to kill the HW.
Not even just as root and for sure not that easy.
For obvious reason, I haven't tested this patch thoroughly.

I have a mix of feelings about the patch.

Long story, short:

- We ought to have a FSL_OTP_WRITE_SUPPORT config option

The writing support is specially useful for manufacturing images where
we can use Linux to do the initial programing, consult a database to
find out inventory information (MAC address for example) and do proper
programing on the OTP fuses. However I also believe we shouldn't have
this available on regular kernel images as it is easy to get a
'expensive paper weight'.

So would you be keen to rework the patch and include a write support
config option?

--
Otavio Salvador O.S. Systems
http://www.ossystems.com.br http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854 Mobile: +1 (347) 903-9750
--

Alexander Holler

2014-11-07 14:40:48 UTC

Post by Alexander Holler
Nothing in userspace should be able to kill the HW.
Not even just as root and for sure not that easy.
For obvious reason, I haven't tested this patch thoroughly.

I have a mix of feelings about the patch.
- We ought to have a FSL_OTP_WRITE_SUPPORT config option
The writing support is specially useful for manufacturing images where
we can use Linux to do the initial programing, consult a database to
find out inventory information (MAC address for example) and do proper
programing on the OTP fuses. However I also believe we shouldn't have
this available on regular kernel images as it is easy to get a
'expensive paper weight'.
So would you be keen to rework the patch and include a write support
config option?

No.

Sorry but I really think such dangerous stuff never should make it's way
into any kernel which doesn't have big red and yellow signs attached to
it. And unfortunately that isn't possible.

And even if it would be possible to mark kernels as dangerous, I think
that functionality should only be part of a bootloader where it isn't
reachable by normal userspace.

Regards,

Alexander Holler
--

Otavio Salvador

2014-11-07 15:06:24 UTC

Post by Alexander Holler
Nothing in userspace should be able to kill the HW.
Not even just as root and for sure not that easy.
For obvious reason, I haven't tested this patch thoroughly.

No.
Sorry but I really think such dangerous stuff never should make it's way
into any kernel which doesn't have big red and yellow signs attached to
it. And unfortunately that isn't possible.
And even if it would be possible to mark kernels as dangerous, I think
that functionality should only be part of a bootloader where it isn't
reachable by normal userspace.

Ok so I am here Nacking your patch.

--
Otavio Salvador O.S. Systems
http://www.ossystems.com.br http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854 Mobile: +1 (347) 903-9750
--

Alexander Holler

2014-11-07 15:26:06 UTC

Post by Alexander Holler
Nothing in userspace should be able to kill the HW.
Not even just as root and for sure not that easy.
For obvious reason, I haven't tested this patch thoroughly.

No.
Sorry but I really think such dangerous stuff never should make it's way
into any kernel which doesn't have big red and yellow signs attached to
it. And unfortunately that isn't possible.
And even if it would be possible to mark kernels as dangerous, I think
that functionality should only be part of a bootloader where it isn't
reachable by normal userspace.

Ok so I am here Nacking your patch.

Feel free to so. Seems to be part of your responsibility.

Alexander Holler
--

Eric Bénard

2014-11-07 14:00:03 UTC

Hi Alexander,

Le Fri, 7 Nov 2014 10:43:34 +0100,

Post by Alexander Holler
Jon Nettleton made me aware that there is a driver which makes it very
easy to write to the OTP from userspace.
And I have to admit that I was mildly shocked.
Because OTP stands for One Time Programmable, it means the write
functionality of that driver can be used only once per HW and if that
is done wrong, the HW might be one of those famous electronic bricks
afterwards.
So I've quickly written a patch which makes this driver to a friendly
citizen instead of an heavily armed cowboy.
I really, really think this patch should be attached to all trees which
are including fsl_otp.c. It applies without any error to
imx_3.10.17_1.0.1_ga, imx_3.10.31_1.1.0_beta, linux-linaro-lsk-v3.14-mx6
and likely any other tree which includes that driver.

That's not shocking to have this feature in a reference design's BSP :
Freescale provides base source code to evaluate their SOC and writing
OTP is one feature of their SOC.
Then that's the end product designer's responsibility to enable/disable
features he wants or not on its product.

The solution proposed by Otavio to add an option to ease the
disabling of this feature is far better than fully removing the code.

Best regards,
Eric
--

Alexander Holler

2014-11-07 14:55:28 UTC

Fair enough. Can we all agree that it should default to disabled?

Anyone who is in need to write the OTP should be able to apply a patch
or to do a git revert in order to use that functionality.

Those which are unable to do so, might not have an idea about OTP at
all, or they might not be aware how dangerous it is to write it (besides
the necessary knowledge about what to change).

Regards,

Alexander Holler

-Jon

Post by Eric BÃ©nard
Hi Alexander,
Le Fri, 7 Nov 2014 10:43:34 +0100,

Freescale provides base source code to evaluate their SOC and writing
OTP is one feature of their SOC.
Then that's the end product designer's responsibility to enable/disable
features he wants or not on its product.
The solution proposed by Otavio to add an option to ease the
disabling of this feature is far better than fully removing the code.
Best regards,
Eric

Otavio Salvador

2014-11-07 15:07:26 UTC

Hi Jon,
Le Fri, 7 Nov 2014 15:31:15 +0100,

Fair enough. Can we all agree that it should default to disabled?

On your product sure, on the evaluation board that's more questionable.
When an evaluation board's user reach this sysfs entries to do a write
to it I believe that's not a random choice but a user's choice.
Maybe adding a otp_unlock entry to the sysfs before allowing write could
secure more the feature when running on the evaluation boards so that
the user can think twice before writing to the fuses.

I like Eric's idea of the unlock sysfs entry for this purpose.

--
Otavio Salvador O.S. Systems
http://www.ossystems.com.br http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854 Mobile: +1 (347) 903-9750
--

Alexander Holler

2014-11-07 15:23:10 UTC

Hi Jon,
Le Fri, 7 Nov 2014 15:31:15 +0100,

Fair enough. Can we all agree that it should default to disabled?

I like Eric's idea of the unlock sysfs entry for this purpose.

Maybe I should mention that this extremly dangerous driver already was
posted for submission into the (production) kernel and nobody had a
problem with it's write functionality, just with some other stuff.

And even if it's ok for FSL to promote this feature with their BSP, I
have absolutely no love for the way it was done. It now is likely part
of many production kernels, something never should have happened.

And I still see absolutely no need to have some one time functionality
as part of the kernel, regardless how usefull it might be for some
developers. It's a waste of time and resources to include that write
functionality in something meant for the greater public. Especially
something that dangerous.

Alexander Holler

--

Otavio Salvador

2014-11-07 16:00:42 UTC

Hello Alexander,

Hi Jon,
Le Fri, 7 Nov 2014 15:31:15 +0100,

Fair enough. Can we all agree that it should default to disabled?

I like Eric's idea of the unlock sysfs entry for this purpose.

Maybe I should mention that this extremly dangerous driver already was
posted for submission into the (production) kernel and nobody had a problem
with it's write functionality, just with some other stuff.
And even if it's ok for FSL to promote this feature with their BSP, I have
absolutely no love for the way it was done. It now is likely part of many
production kernels, something never should have happened.
And I still see absolutely no need to have some one time functionality as
part of the kernel, regardless how usefull it might be for some developers.
It's a waste of time and resources to include that write functionality in
something meant for the greater public. Especially something that dangerous.

We have two different things here.

* Linux support: the driver is perfectly capable of handling the
reading and writing
* Driver settings: We should, indeed, have it to be explicitly
enabled on the kernel config to allow writing

This is done in other areas of kernel (one which comes to my mind is
NTFS Writing support) and what I proposed was you to make:

CONFIG_FSL_OTP -> read-only
CONFIG_FSL_OTP_WRITE -> write support

The write support needs to be enabled on mfgtools defconfig and
disabled on regular kernels (this is what I think it should be but it
is up to the board maintainer).

--
Otavio Salvador O.S. Systems
http://www.ossystems.com.br http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854 Mobile: +1 (347) 903-9750
--

Alexander Holler

2014-11-07 16:38:14 UTC

Post by Otavio Salvador
Hello Alexander,

Hi Jon,
Le Fri, 7 Nov 2014 15:31:15 +0100,

Fair enough. Can we all agree that it should default to disabled?

I like Eric's idea of the unlock sysfs entry for this purpose.

Maybe I should mention that this extremly dangerous driver already was
posted for submission into the (production) kernel and nobody had a problem
with it's write functionality, just with some other stuff.
And even if it's ok for FSL to promote this feature with their BSP, I have
absolutely no love for the way it was done. It now is likely part of many
production kernels, something never should have happened.
And I still see absolutely no need to have some one time functionality as
part of the kernel, regardless how usefull it might be for some developers.
It's a waste of time and resources to include that write functionality in
something meant for the greater public. Especially something that dangerous.

We have two different things here.
* Linux support: the driver is perfectly capable of handling the
reading and writing
* Driver settings: We should, indeed, have it to be explicitly
enabled on the kernel config to allow writing
This is done in other areas of kernel (one which comes to my mind is
CONFIG_FSL_OTP -> read-only
CONFIG_FSL_OTP_WRITE -> write support
The write support needs to be enabled on mfgtools defconfig and
disabled on regular kernels (this is what I think it should be but it
is up to the board maintainer).

Sorry, but based on my experience and confrontations with Murphy I can't
agree.

To conclude what already happened:

- Someone has written that driver in the way it is.
- Several devs likely have seen it before it made it's way into the BSP.
- It got enabled by default in the BSP (defconfigs).
- It made it's way into YOCTO.
- It was submitted for inclusion into the kernel.

So the driver already was seen by a lot of people (most likely all devs)
and still nobody has seen a problem. And most of these people did know
more about its functionality than most people which are just changing
switches in kernel configs or do play with sysfs.

And because I don't like to play the bad boy, I'm not going to discuss
this any further. I did my part as responsible developer and I think I
made my standpoint clear. I hoped it would have happened more silently
without that discussion, but ... ;)

But feel free to continue this discussion, I don't care if my patch will
be included or not. I just will not modify it. ;)

Regards,

Alexander Holler

--

Nikolay Dimitrov

2014-11-08 02:03:08 UTC

Hi Alexander,

The driver allows to be enabled/disabled by a configuration option, so
it's a responsibility of your engineering team to properly configure
the software for development, manufacturing and production purposes, as
there's no "one size fits all" solution for this option.

And I think it would be a unwise decision just to cripple the driver
because there is potential for misuse the driver. If so, we have to
disable also all device files and sysfs entries that allow raw access
to physical memory, physical disks, cpu frequency, thermal device,
power supply voltages, as all of these can screw-up the product (either
by deleting data or by frying a component on the board). And we have to
forbid kitchen knives :).

I like Eric's idea with sysfs unlock entry. It's also possible to have
different sysfs "read" and "write" files' permissions, to provide
privilege separation.

I also understand your confusion of the answers received on LKML and
here, but you should already know that each FOSS tribe has its own
customs. What's good for the kernel itself doesn't make it instantly
good for actual systems/product integration, so it's normal to have
groups with different goals to react differently on the same topic.

Kind regards,
Nikolay
--

Jon Nettleton

2014-11-08 09:32:25 UTC

Post by Eric BÃ©nard
Hi Alexander,
The driver allows to be enabled/disabled by a configuration option, so
it's a responsibility of your engineering team to properly configure
the software for development, manufacturing and production purposes, as
there's no "one size fits all" solution for this option.
And I think it would be a unwise decision just to cripple the driver
because there is potential for misuse the driver. If so, we have to
disable also all device files and sysfs entries that allow raw access
to physical memory, physical disks, cpu frequency, thermal device,
power supply voltages, as all of these can screw-up the product (either
by deleting data or by frying a component on the board). And we have to
forbid kitchen knives :).
I like Eric's idea with sysfs unlock entry. It's also possible to have
different sysfs "read" and "write" files' permissions, to provide
privilege separation.
I also understand your confusion of the answers received on LKML and
here, but you should already know that each FOSS tribe has its own
customs. What's good for the kernel itself doesn't make it instantly
good for actual systems/product integration, so it's normal to have
groups with different goals to react differently on the same topic.

As has already been mentioned, it is the responsibility of a project to ensure that any product is suitably protected.
I can image a customer would be really unhappy if their system (e.g. Smart TV) could be destroyed by a bit of malware !

Yes but many of the boards aren't necessarily being used for locked
down consumer products. Many people are using these devices as
learning devices and are trusting the code provided by the
manufacturer. I think it makes sense for public software to try and
provide the most sane configuration. It is trivial to make this a
kernel config option that is disabled by default, add a write_enabled
sysfs node, and even have it spit out a warning when you enable the
write_enabled sysfs node. If we do all those things and somebody
bricks their board then I would not have a bad conscience.

Anybody that has the skill or reason to use the fuses properly will
have no problem following steps to enable this functionality.
--

Alexander Holler

2014-11-08 18:49:55 UTC

Sorry to disappoint you, but I'm currently just a hobbyist. ;)

Post by Eric BÃ©nard
And I think it would be a unwise decision just to cripple the driver
because there is potential for misuse the driver. If so, we have to
disable also all device files and sysfs entries that allow raw access
to physical memory, physical disks, cpu frequency, thermal device,
power supply voltages, as all of these can screw-up the product (either
by deleting data or by frying a component on the board). And we have to
forbid kitchen knives :).

Hmm, maybe I should repeat that this driver is about ONE TIME
PROGRAMMABLE memory. So all your comparisons are (imho) totally wrong.

YOU ONLY HAVE ONE SHOT to set one of these values to whatever value you
want. And if you get it wrong, the HW might be unusable afterwards. Not
to speak about all the things which can go wrong and might write to one
of these files, changing the (hopefully correct) values such that the HW
is dead afterwards too.

Post by Eric BÃ©nard
I like Eric's idea with sysfs unlock entry. It's also possible to have
different sysfs "read" and "write" files' permissions, to provide
privilege separation.
I also understand your confusion of the answers received on LKML and
here, but you should already know that each FOSS tribe has its own
customs. What's good for the kernel itself doesn't make it instantly
good for actual systems/product integration, so it's normal to have
groups with different goals to react differently on the same topic.

I'm not confused, at least not in regard what you want to suggest. Of
course, I'm totally confused about the fact that almost nobody else
before has critized that write functionality of this driver, also I'm
already used to the fact that I'm unable to understand many things which
are happing in todays world. But nobody is perfect. ;)

But there is absolutely no reason to include this ONE TIME FUNCTIONALITY
into any kernel meant for the public, especially as it is very
dangerous. And for sure not without any content veryfication and some
other means to make sure the value one wants to put ONCE into that ONE
TIME MEMORY isn't wrong.

That's why I just refuse to change my patch in order to add write
functionality. I don't want to do (imho) foolish things just because
someone else tells me to do them. I'm no Lemming.

At least not without adequate compensations. ;)

Regards,

Alexander Holler
--

Alexander Holler

2014-11-09 10:14:39 UTC

Post by Alexander Holler
I'm not confused, at least not in regard what you want to suggest. Of
course, I'm totally confused about the fact that almost nobody else
before has critized that write functionality of this driver, also I'm
already used to the fact that I'm unable to understand many things which
are happing in todays world. But nobody is perfect. ;)

Just to make it more clear what this thread is about, here is a relevant
sentence copied form the reference manual for the chip:

"In order to avoid "rogue" code performing erroneous writes to OTP, a
special unlocking sequence is required for writes to the fuse banks."

Now guess why the HW was designed this way. And then look again at the
driver which nullifies the careful implementation in the HW. It doesn't
have to be the fault of the author, e.g. he just might have written it
for internal use. The problem is that it went into kernels for public
use and nobody has seen a problem. Might be because of missing knowledge
about what the driver does at all or whatever. I don't know.

Alexander Holler
--

Eric Bénard

2014-11-09 15:09:25 UTC

Hi Alexander,

Le Sun, 09 Nov 2014 11:14:39 +0100,

Just to make it more clear what this thread is about, here is a relevant
"In order to avoid "rogue" code performing erroneous writes to OTP, a
special unlocking sequence is required for writes to the fuse banks."

That's why adding an unlock sysfs entry would match the required
sequence to unlock the write access on the user point of view, but
that's Freescale's problem and policy.

Post by Alexander Holler
Now guess why the HW was designed this way. And then look again at the
driver which nullifies the careful implementation in the HW. It doesn't
have to be the fault of the author, e.g. he just might have written it
for internal use. The problem is that it went into kernels for public
use and nobody has seen a problem. Might be because of missing knowledge
about what the driver does at all or whatever. I don't know.

Evaluation boards come with unlocked/unburned fuse so if a designer
wants to evaluate his fuse configuration on an EVB during the design
phase of his custom product he may need this driver especially when
using Freescale's MFG Tools.
If your Wandboard has unburned fuses and their kernel has this driver,
then ask them why they keep this features open to their users by default
(this can be because the SOM are intended to be used in final products
and they let the final user of their SOM burn what he wants in the fuse
- his own MAC address for example - so here again that's the job of the
end product designer to remove this feature once he launch his product
on the market).

Best regards,
Eric
--

Bob Cochran

2014-11-09 17:03:25 UTC

Post by Eric BÃ©nard
Hi Alexander,
Le Sun, 09 Nov 2014 11:14:39 +0100,

That's why adding an unlock sysfs entry would match the required
sequence to unlock the write access on the user point of view, but
that's Freescale's problem and policy.

Hi,

My preference would be to have an otp recipe under
meta-fsl-arm/recipes-manufacturing and in the recipe would be the otp
kernel patch that could be applied along with user space code to
exercise it.

As an un-applied patch, I would never have to worry about it bricking my
board. However, when it comes time to use the feature in a
manufacturing environment, it's there for me.

And for extra safety, give the patch an unusual suffix (e.g., .otp) so
it doesn't accidentally get swept into a kernel patch set.

Bob

Post by Eric BÃ©nard

Nikolay Dimitrov

2014-11-09 12:34:25 UTC

Hi Alexander,

The eFuses are lockable. If you don't intend to further modify their
value after proper programming, you should lock them. If you don't do
so, you shouldn't blame the Linux driver for the consequences.

Crippling the IMX OTP driver doesn't solve the system security issues.
I can write to the IMX physical memory (e.g. to program OCOTP
registers) without this Linux driver at all, I just need proper
privileges and the devregs tool (thanks Eric & Troy!).

I have just like you some imx6 hobby boards. If one of them is bricked,
it's totally my fault. And yet we have hundreds of thousands of imx6
boards on the road, which we locked during manufacturing, and disabled
the OTP driver, so it's impossible to brick them via OTP.

To me it seems that the current state of affairs is already perfectly
OK and there's no need to panic.

Kind regards,
Nikolay
--

Alexander Holler

2014-11-09 18:09:22 UTC

Post by Eric BÃ©nard
Hi Alexander,
The eFuses are lockable. If you don't intend to further modify their
value after proper programming, you should lock them. If you don't do
so, you shouldn't blame the Linux driver for the consequences.
Crippling the IMX OTP driver doesn't solve the system security issues.
I can write to the IMX physical memory (e.g. to program OCOTP
registers) without this Linux driver at all, I just need proper
privileges and the devregs tool (thanks Eric & Troy!).
I have just like you some imx6 hobby boards. If one of them is bricked,
it's totally my fault. And yet we have hundreds of thousands of imx6
boards on the road, which we locked during manufacturing, and disabled
the OTP driver, so it's impossible to brick them via OTP.

First let me quote again the sentence from the reference manual you
missed to quote:

"In order to avoid "rogue" code performing erroneous writes to OTP, a
special unlocking sequence is required for writes to the fuse banks."

To conclude, you are saying:

- The (easy) write functionalty of the driver should be kept because
it's of no use on locked devices anyway.

- Unlocked devices ("hobby boards", whatever that is) are second class
devices where it's the owners fault if some malware, wrong script,
typo or similiar writes to some file in the sysfs.

- The way to write to the OTP offered by the driver is the same as the
way offered by the HW, which means the more complicated way
implemented in the HW (which, by the way, is still in effect when
using devregs) is senseless.

...

Post by Eric BÃ©nard
To me it seems that the current state of affairs is already perfectly
OK and there's no need to panic.

So revealing and trying to fix bugs nowadays means panicking?

I thought it's still part of the normal software development process.
Maybe I really have become oldschool and better get a towel.

Anyway, as I'm running out of hope and polite arguments (at least in
regard to this topic) and getting near to the point of using sentences
Linus has unfortunately become famous for, I better stop to comment
further.

Alexander Holler
--

Nikolay Dimitrov

2014-11-09 19:20:07 UTC

Hi Alexander,

I really appreciate your efforts to explain your motivation for the
patch, and also respect your position, even if we didn't reached
consensus.

Thanks to everyone who contributed valuable thoughts on subject. I feel
that can't contribute anything else to the topic, so please excuse me.

Kind regards,
Nikolay
--

Eric Bénard

2014-11-07 16:03:17 UTC

Hi Alexander,

Le Fri, 07 Nov 2014 16:23:10 +0100,

Hi Jon,
Le Fri, 7 Nov 2014 15:31:15 +0100,

Fair enough. Can we all agree that it should default to disabled?

I like Eric's idea of the unlock sysfs entry for this purpose.

In LAKML there was a post for a driver (for 23/28 at the moment) where
write support was dropped :
http://comments.gmane.org/gmane.linux.drivers.devicetree/94343

In mainline kernel :
- on the i.MX there is only one function used to get the MAC :
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/mach-mxs/mach-mxs.c#n90
- read only fuse infrastructure was integrated for Tegra :
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/soc/tegra/fuse

In Freescale's released kernel, the feature is available but one more
time I think that's not a production kernel but a base source code to
evaluate SOC's features on an evaluation board and then design
your own products where it's your responsibility to add / remove
features.

Post by Alexander Holler
And even if it's ok for FSL to promote this feature with their BSP, I
have absolutely no love for the way it was done. It now is likely part
of many production kernels, something never should have happened.

It's the developer's responsibility to add/remove features on _their_
production kernel for _their_ product.

Post by Alexander Holler
And I still see absolutely no need to have some one time functionality
as part of the kernel, regardless how usefull it might be for some
developers. It's a waste of time and resources to include that write
functionality in something meant for the greater public. Especially
something that dangerous.

If you look how Freescale has designed their "Manufacturing tool" and
check a few of their xml files, you will understand why they need this
kind of low level access features in the kernel ;-)

Here on real products we do all that at bootloader level using a
special production version of the bootloader but that's an other story.

Best regards,
Eric
--

Eric Bénard

2014-11-07 15:50:13 UTC

Hi Otavio,

Le Fri, 7 Nov 2014 13:07:26 -0200,

Maybe adding a otp_unlock entry to the sysfs before allowing write could
secure more the feature when running on the evaluation boards so that
the user can think twice before writing to the fuses.

I like Eric's idea of the unlock sysfs entry for this purpose.

FWIW that's how it's done in barebox :
http://git.pengutronix.de/?p=barebox.git;a=blob;f=arch/arm/mach-imx/ocotp.c;h=837500fff8865d64ea2ed0f129cb2a8869e0aa7d;hb=HEAD#l430

http://git.pengutronix.de/?p=barebox.git;a=blob;f=arch/arm/mach-imx/ocotp.c;h=837500fff8865d64ea2ed0f129cb2a8869e0aa7d;hb=HEAD#l294

Eric
--

Robin Findley

2014-11-08 21:54:35 UTC

From Alexander Holler

Post by Alexander Holler
But there is absolutely no reason to include this ONE TIME FUNCTIONALITY
into any kernel meant for the public, especially as it is very
dangerous.

The problem isn't that someone can burn fuses in a commercial product.
Rather, the problem is a designer who ships a product with unburned fuses. If
a designer is unaware of the fuses, and ships them unburned (unless he has
good reason), then he shouldn't be selling commercial products. You can't
design an imx product without knowing about the fuses. They are fundamental
to the design process.

I agree that a config option might be useful, and maybe even disabled by
default. But for manufacturing purposes, having the ability to burn the fuses
during production testing with production software is a good thing. And once
they are burned, the proposed nightmare scenarios no longer exist, and it
doesn't matter whether the kernel has the option enabled.

Post by Alexander Holler
And for sure not without any content veryfication and some
other means to make sure the value one wants to put ONCE into that ONE
TIME MEMORY isn't wrong.
That's why I just refuse to change my patch in order to add write
functionality. I don't want to do (imho) foolish things just because
someone else tells me to do them. I'm no Lemming.
At least not without adequate compensations. ;)
Regards,
Alexander Holler

Alexander Holler

2014-11-10 14:11:44 UTC

Post by Robin Findley
From Alexander Holler

Post by Alexander Holler
But there is absolutely no reason to include this ONE TIME FUNCTIONALITY
into any kernel meant for the public, especially as it is very
dangerous.

You're only talking about locked products which are including the SW, do
you?

What's if the software isn't part of you manufacturing process and you
want to leave the customer the choice to enter secure mode whenever he wish?

Setting and locking fuses means removing options and crippling the HW.
That's their only purpose.

Alexander Holler
--

Alexander Holler

2014-11-13 19:19:49 UTC