Message ID | 6cc4810c-1e2e-4ebf-912a-96936f035c0e@mary.at.omicron.at |
---|---|
State | Rejected |
Delegated to: | Wolfgang Denk |
Headers | show |
Dear Christian Riesch, In message <6cc4810c-1e2e-4ebf-912a-96936f035c0e@mary.at.omicron.at> you wrote: > Signed-off-by: Christian Riesch <christian.riesch@omicron.at> > --- > board/omicron/calimain/calimain.c | 31 ++++++++++++++++++++++++++++++- > include/configs/calimain.h | 2 ++ > 2 files changed, 32 insertions(+), 1 deletion(-) > > diff --git a/board/omicron/calimain/calimain.c b/board/omicron/calimain/calimain.c > index 1060a1f..80e3893 100644 > --- a/board/omicron/calimain/calimain.c > +++ b/board/omicron/calimain/calimain.c > @@ -1,5 +1,5 @@ > /* > - * Copyright (C) 2011 OMICRON electronics GmbH > + * Copyright (C) 2011-2013 OMICRON electronics GmbH > * > * Based on da850evm.c. Original Copyrights follow: > * > @@ -136,6 +136,35 @@ int board_init(void) > return 0; > } > > +/* seed random number generator with uninitialized SRAM content */ > +static void srand_sram(void) > +{ > + int *p; > + int seed = 0; > + > + for (p = (int *) 0x80000000; p < (int *) 0x8001ffff; p++) > + seed ^= *p; > + > + srand(seed); > +} Note that your "uninitialized" SRAM content is probably not so much random at all - I guess, it is much less random than the originally used timer value. What exactly is your justification for such a change? Please elucidate... > +int board_late_init(void) > +{ > + uchar enetaddr[6]; > + > + if (!eth_getenv_enetaddr("ethaddr", enetaddr)) { > + srand_sram(); > + eth_random_enetaddr(enetaddr); > + if (eth_setenv_enetaddr("ethaddr", enetaddr)) { > + printf("Failed to set random ethernet address\n"); > + } else { > + printf("Setting random ethernet address %pM.\n", > + enetaddr); > + } > + } > + return 0; > +} NAK! You are but duplicating the code already present in net/eth.c This makes no sense. Best regards, Wolfgang Denk
Hello Wolfgang, Thank you again for your comments. On 2013-01-08 18:39, Wolfgang Denk wrote: > Dear Christian Riesch, > > In message <6cc4810c-1e2e-4ebf-912a-96936f035c0e@mary.at.omicron.at> you wrote: >> Signed-off-by: Christian Riesch <christian.riesch@omicron.at> >> --- >> board/omicron/calimain/calimain.c | 31 ++++++++++++++++++++++++++++++- >> include/configs/calimain.h | 2 ++ >> 2 files changed, 32 insertions(+), 1 deletion(-) >> >> diff --git a/board/omicron/calimain/calimain.c b/board/omicron/calimain/calimain.c >> index 1060a1f..80e3893 100644 >> --- a/board/omicron/calimain/calimain.c >> +++ b/board/omicron/calimain/calimain.c >> @@ -1,5 +1,5 @@ >> /* >> - * Copyright (C) 2011 OMICRON electronics GmbH >> + * Copyright (C) 2011-2013 OMICRON electronics GmbH >> * >> * Based on da850evm.c. Original Copyrights follow: >> * >> @@ -136,6 +136,35 @@ int board_init(void) >> return 0; >> } >> >> +/* seed random number generator with uninitialized SRAM content */ >> +static void srand_sram(void) >> +{ >> + int *p; >> + int seed = 0; >> + >> + for (p = (int *) 0x80000000; p < (int *) 0x8001ffff; p++) >> + seed ^= *p; >> + >> + srand(seed); >> +} > > Note that your "uninitialized" SRAM content is probably not so much > random at all - There are several papers around describing the use of initial SRAM value after power up for the generation of random numbers. This is why I gave it a try, and it works pretty well for me. I get a different seed for each power-up cycle. I guess that the randomness is limited and that part of the generated seed is more a fingerprint for the chip, therefore it may not be good enough for security related stuff, but for my purpose it's ok. > I guess, it is much less random than the originally > used timer value. In my case the timer value is not random at all since it is reset to zero at power-up. Since seeding the random number generator is always done at the same time after power-up in the current code, the seed will always be the same for my devices. Therefore the generated MAC address will always be the same for all devices. > What exactly is your justification for such a change? Please > elucidate... Actually I do not change anything ;-) For the lsxl board that is currently the only user of eth_random_enet(), nothing changes. get_timer(0) remains the source of the randomness for this board. My patches only allow other boards to use a sources of randomness that is available to them instead of forcing everyone to use get_timer(0). > >> +int board_late_init(void) >> +{ >> + uchar enetaddr[6]; >> + >> + if (!eth_getenv_enetaddr("ethaddr", enetaddr)) { >> + srand_sram(); >> + eth_random_enetaddr(enetaddr); >> + if (eth_setenv_enetaddr("ethaddr", enetaddr)) { >> + printf("Failed to set random ethernet address\n"); >> + } else { >> + printf("Setting random ethernet address %pM.\n", >> + enetaddr); >> + } >> + } >> + return 0; >> +} > > NAK! You are but duplicating the code already present in net/eth.c Apparently I am missing something here. I do not see a call of eth_random_enetaddr() in net/eth.c. To which part of net/eth.c are you referring? Regards, Christian > This makes no sense. > > Best regards, > > Wolfgang Denk >
Dear Christian Riesch, In message <50ED2B84.2080905@omicron.at> you wrote: > > > What exactly is your justification for such a change? Please > > elucidate... > > Actually I do not change anything ;-) Why do you need a patch then? :-P > > NAK! You are but duplicating the code already present in net/eth.c > > Apparently I am missing something here. I do not see a call of > eth_random_enetaddr() in net/eth.c. To which part of net/eth.c are you > referring? Hm... I can't find it now, either. Dunno what I have seen then. eventually I was looking at your patch in two windows :-( Sorry - but still: very similar code exists in rescue_mode() in "board/buffalo/lsxl/lsxl.c"; this should be factored out. Best regards, Wolfgang Denk
diff --git a/board/omicron/calimain/calimain.c b/board/omicron/calimain/calimain.c index 1060a1f..80e3893 100644 --- a/board/omicron/calimain/calimain.c +++ b/board/omicron/calimain/calimain.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2011 OMICRON electronics GmbH + * Copyright (C) 2011-2013 OMICRON electronics GmbH * * Based on da850evm.c. Original Copyrights follow: * @@ -136,6 +136,35 @@ int board_init(void) return 0; } +/* seed random number generator with uninitialized SRAM content */ +static void srand_sram(void) +{ + int *p; + int seed = 0; + + for (p = (int *) 0x80000000; p < (int *) 0x8001ffff; p++) + seed ^= *p; + + srand(seed); +} + +int board_late_init(void) +{ + uchar enetaddr[6]; + + if (!eth_getenv_enetaddr("ethaddr", enetaddr)) { + srand_sram(); + eth_random_enetaddr(enetaddr); + if (eth_setenv_enetaddr("ethaddr", enetaddr)) { + printf("Failed to set random ethernet address\n"); + } else { + printf("Setting random ethernet address %pM.\n", + enetaddr); + } + } + return 0; +} + #ifdef CONFIG_DRIVER_TI_EMAC /* * Initializes on-board ethernet controllers. diff --git a/include/configs/calimain.h b/include/configs/calimain.h index 5c2b35d..8cea0d9 100644 --- a/include/configs/calimain.h +++ b/include/configs/calimain.h @@ -30,6 +30,7 @@ #define CONFIG_DRIVER_TI_EMAC #define MACH_TYPE_CALIMAIN 3528 #define CONFIG_MACH_TYPE MACH_TYPE_CALIMAIN +#define CONFIG_BOARD_LATE_INIT /* * SoC Configuration @@ -202,6 +203,7 @@ #define CONFIG_BOOTP_DNS2 #define CONFIG_BOOTP_SEND_HOSTNAME #define CONFIG_NET_RETRY_COUNT 10 +#define CONFIG_RANDOM_MACADDR #endif /*
Signed-off-by: Christian Riesch <christian.riesch@omicron.at> --- board/omicron/calimain/calimain.c | 31 ++++++++++++++++++++++++++++++- include/configs/calimain.h | 2 ++ 2 files changed, 32 insertions(+), 1 deletion(-)