Subject: Re: [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr

From: Dan Carpenter <[email-masked]>
Date: Thu, 3 Mar 2022 11:37:16 +0300

On Wed, Mar 02, 2022 at 12:07:04PM -0800, Kees Cook wrote:
> On Wed, Mar 02, 2022 at 10:29:31AM +0100, Rasmus Villemoes wrote:
> > This won't help the current issue (because it doesn't exist and might
> > never), but just in case some compiler people are listening, I'd like to
> > have some sort of way to tell the compiler "treat this variable as
> > uninitialized from here on". So one could do
> > 
> > #define kfree(p) do { __kfree(p); __magic_uninit(p); } while (0)
> > 
> > with __magic_uninit being a magic no-op that doesn't affect the
> > semantics of the code, but could be used by the compiler's "[is/may be]
> > used uninitialized" machinery to flag e.g. double frees on some odd
> > error path etc. It would probably only work for local automatic
> > variables, but it should be possible to just ignore the hint if p is
> > some expression like foo->bar or has side effects. If we had that, the
> > end-of-loop test could include that to "uninitialize" the iterator.
> 
> I've long wanted to change kfree() to explicitly set pointers to NULL on
> free. https://github.com/KSPP/linux/issues/87 

You also need to be a bit careful with existing code because there are
places which do things like:

drivers/usb/host/r8a66597-hcd.c
   424          kfree(dev);
                      ^^^
   425  
   426          for (port = 0; port < r8a66597->max_root_hub; port++) {
   427                  if (r8a66597->root_hub[port].dev == dev) {
                                                            ^^^
   428                          r8a66597->root_hub[port].dev = NULL;
   429                          break;
   430                  }
   431          }

Printing the freed pointer in debug code is another thing people do.

regards,
dan carpenter