Re: RFC for a new string-copy function, using mixtures of strlcpy and strscpy

[Ajay Garg]

On Mon, Nov 8, 2021 at 6:31 PM Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On Mon, Nov 08, 2021 at 06:15:54PM +0530, Ajay Garg wrote:
> > Hi Greg,
> >
> > Thanks for your time.
> >
> > >
> > > Wait, why?  We have recently added new string copy functions to resolve
> > > the type of issues you have pointed out.  The kernel is not yet
> > > converted to use them, so why add yet-another-function that also is not
> > > used?
> >
> > Greg, would request your couple of minutes more, in suggesting a
> > concrete way forward, by working through an example as below.
> >
> >
> > In the file fs/kernfs/dir.c, there is a statement
> >
> >         return strlcpy(buf, kn->parent ? kn->name : "/", buflen);
> >
> > Here, there is no information of how long kn->name might be, so there
> > is a definite chance of overflow happening. Thus, accordingly, strlcpy
> > is used, to bound copying of upto buflen bytes. Of course, buf
> > (destination-buffer) is safe from any overflow now.
>
> We "know" that kn->name will not overflow here based on the callers of
> this function, right?  I can not find any caller that passes in a buffer
> that would overflow, or am I missing a callpath somewhere?
>
> > However, iffff strlen(kn->name) is greater than (buflen - 1), then
> > strlcpy would return a different value than the number of bytes
> > actually copied. Since there is no check, this (wrong) return value
> > will be propagated to the clients down the stack.
>
> But in the existing kernel, can that happen today?  I can't find any
> caller that would be using this in that manner.
>
> And that's what matters.  Not the theoretical callers in the possible
> future, but the in-kernel users of the functions today.
>
> If these calls are wrong, then they should be fixed, but adding another
> string function to the heap of ones we have should only be done after
> considering the ones that we currently have, why they were added, and
> why existing code has not yet been converted to use them.
>

Got it, thanks Greg !


Thanks and Regards,
Ajay