Re: [PATCH 1/3] lib: early_string: allow early usage of some string functions

2021-05-03 Thread Daniel Walker
On Mon, May 03, 2021 at 11:01:41AM -0700, Daniel Walker wrote:
> On Sat, May 01, 2021 at 09:31:47AM +0200, Christophe Leroy wrote:
> > 
> > > In fact, should be like in prom_init today:
> > > 
> > > #ifdef __EARLY_STRING_ENABLED
> > >  if (dsize >= count)
> > >      return count;
> > > #else
> > >  BUG_ON(dsize >= count);
> > > #endif
> > 
> > Thinking about it once more, this BUG_ON() is overkill and should be
> > avoided, see https://www.kernel.org/doc/html/latest/process/deprecated.html
> > 
> > Therefore, something like the following would make it:
> > 
> > if (dsize >= count) {
> > WARN_ON(!__is_defined(__EARLY_STRING_ENABLED));
> > 
> > return count;
> > }
> 
> I agree, it's overkill it stop the system for this condition.
> 
> how about I do something more like this for my changes,
> 
> 
> > if (WARN_ON(dsize >= count && !__is_defined(__EARLY_STRING_ENABLED)))
> > return count;

I'll have to work on this one..

Daniel


Re: [PATCH 1/3] lib: early_string: allow early usage of some string functions

2021-05-03 Thread Daniel Walker
On Sat, May 01, 2021 at 09:31:47AM +0200, Christophe Leroy wrote:
> 
> > In fact, should be like in prom_init today:
> > 
> > #ifdef __EARLY_STRING_ENABLED
> >  if (dsize >= count)
> >      return count;
> > #else
> >  BUG_ON(dsize >= count);
> > #endif
> 
> Thinking about it once more, this BUG_ON() is overkill and should be
> avoided, see https://www.kernel.org/doc/html/latest/process/deprecated.html
> 
> Therefore, something like the following would make it:
> 
>   if (dsize >= count) {
>   WARN_ON(!__is_defined(__EARLY_STRING_ENABLED));
> 
>   return count;
>   }

I agree, it's overkill it stop the system for this condition.

how about I do something more like this for my changes,


>   if (WARN_ON(dsize >= count && !__is_defined(__EARLY_STRING_ENABLED)))
>   return count;

and for generic kernel,

>   if (WARN_ON(dsize >= count))
>   return count;



Daniel


Re: [PATCH 1/3] lib: early_string: allow early usage of some string functions

2021-05-01 Thread Christophe Leroy




Le 30/04/2021 à 10:50, Christophe Leroy a écrit :



Le 30/04/2021 à 10:47, Christophe Leroy a écrit :



Le 30/04/2021 à 06:22, Daniel Walker a écrit :

This systems allows some string functions to be moved into
lib/early_string.c and they will be prepended with "early_" and compiled
without debugging like KASAN.

This is already done on x86 for,
"AMD Secure Memory Encryption (SME) support"

and on powerpc prom_init.c , and EFI's libstub.

The AMD memory feature disabled KASAN for all string functions, and
prom_init.c and efi libstub implement their own versions of the
functions.

This implementation allows sharing of the string functions without
removing the debugging features for the whole system.


This looks good. I prefer that rather than the way you proposed to do it two 
years ago.

Only one problem, see below.


+size_t strlcat(char *dest, const char *src, size_t count)
+{
+    size_t dsize = strlen(dest);
+    size_t len = strlen(src);
+    size_t res = dsize + len;
+
+    /* This would be a bug */
+    BUG_ON(dsize >= count);


powerpc is not ready to handle BUG_ON() in when in prom_init.

Can you do:

#ifndef __EARLY_STRING_ENABLED
 BUG_ON(dsize >= count);
#endif


In fact, should be like in prom_init today:

#ifdef __EARLY_STRING_ENABLED
 if (dsize >= count)
     return count;
#else
 BUG_ON(dsize >= count);
#endif


Thinking about it once more, this BUG_ON() is overkill and should be avoided, see 
https://www.kernel.org/doc/html/latest/process/deprecated.html


Therefore, something like the following would make it:

if (dsize >= count) {
WARN_ON(!__is_defined(__EARLY_STRING_ENABLED));

return count;
}







+
+    dest += dsize;
+    count -= dsize;
+    if (len >= count)
+    len = count-1;
+    memcpy(dest, src, len);
+    dest[len] = 0;
+    return res;
+}
+EXPORT_SYMBOL(strlcat);
+#endif
+


Re: [PATCH 1/3] lib: early_string: allow early usage of some string functions

2021-04-30 Thread Christophe Leroy




Le 30/04/2021 à 10:47, Christophe Leroy a écrit :



Le 30/04/2021 à 06:22, Daniel Walker a écrit :

This systems allows some string functions to be moved into
lib/early_string.c and they will be prepended with "early_" and compiled
without debugging like KASAN.

This is already done on x86 for,
"AMD Secure Memory Encryption (SME) support"

and on powerpc prom_init.c , and EFI's libstub.

The AMD memory feature disabled KASAN for all string functions, and
prom_init.c and efi libstub implement their own versions of the
functions.

This implementation allows sharing of the string functions without
removing the debugging features for the whole system.


This looks good. I prefer that rather than the way you proposed to do it two 
years ago.

Only one problem, see below.


+size_t strlcat(char *dest, const char *src, size_t count)
+{
+    size_t dsize = strlen(dest);
+    size_t len = strlen(src);
+    size_t res = dsize + len;
+
+    /* This would be a bug */
+    BUG_ON(dsize >= count);


powerpc is not ready to handle BUG_ON() in when in prom_init.

Can you do:

#ifndef __EARLY_STRING_ENABLED
 BUG_ON(dsize >= count);
#endif


In fact, should be like in prom_init today:

#ifdef __EARLY_STRING_ENABLED
if (dsize >= count)
return count;
#else
BUG_ON(dsize >= count);
#endif





+
+    dest += dsize;
+    count -= dsize;
+    if (len >= count)
+    len = count-1;
+    memcpy(dest, src, len);
+    dest[len] = 0;
+    return res;
+}
+EXPORT_SYMBOL(strlcat);
+#endif
+


Re: [PATCH 1/3] lib: early_string: allow early usage of some string functions

2021-04-30 Thread Christophe Leroy




Le 30/04/2021 à 06:22, Daniel Walker a écrit :

This systems allows some string functions to be moved into
lib/early_string.c and they will be prepended with "early_" and compiled
without debugging like KASAN.

This is already done on x86 for,
"AMD Secure Memory Encryption (SME) support"

and on powerpc prom_init.c , and EFI's libstub.

The AMD memory feature disabled KASAN for all string functions, and
prom_init.c and efi libstub implement their own versions of the
functions.

This implementation allows sharing of the string functions without
removing the debugging features for the whole system.


This looks good. I prefer that rather than the way you proposed to do it two 
years ago.

Only one problem, see below.


+size_t strlcat(char *dest, const char *src, size_t count)
+{
+   size_t dsize = strlen(dest);
+   size_t len = strlen(src);
+   size_t res = dsize + len;
+
+   /* This would be a bug */
+   BUG_ON(dsize >= count);


powerpc is not ready to handle BUG_ON() in when in prom_init.

Can you do:

#ifndef __EARLY_STRING_ENABLED
BUG_ON(dsize >= count);
#endif



+
+   dest += dsize;
+   count -= dsize;
+   if (len >= count)
+   len = count-1;
+   memcpy(dest, src, len);
+   dest[len] = 0;
+   return res;
+}
+EXPORT_SYMBOL(strlcat);
+#endif
+