Re: [PATCH] replace incorrect strscpy use in FORTIFY_SOURCE

From: J . Bruce Fields
Date: Tue Jul 18 2017 - 10:54:11 EST


On Fri, Jul 14, 2017 at 04:51:31PM -0700, Kees Cook wrote:
> On Fri, Jul 14, 2017 at 2:28 PM, Daniel Micay <danielmicay@xxxxxxxxx> wrote:
> > Using strscpy was wrong because FORTIFY_SOURCE is passing the maximum
> > possible size of the outermost object, but strscpy defines the count
> > parameter as the exact buffer size, so this could copy past the end of
> > the source. This would still be wrong with the planned usage of
> > __builtin_object_size(p, 1) for intra-object overflow checks since it's
> > the maximum possible size of the specified object with no guarantee of
> > it being that large.
> >
> > Reuse of the fortified functions like this currently makes the runtime
> > error reporting less precise but that can be improved later on.
> >
> > Signed-off-by: Daniel Micay <danielmicay@xxxxxxxxx>
>
> Thanks for fixing this! Linus, do you want to take this directly or
> have it go via -mm where fortify landed originally?
>
> Acked-by: Kees Cook <keescook@xxxxxxxxxxxx>
>
> As far as testing goes, was the NFS tree not in -next, or was a test
> not running against -next? I'm curious why it took until the NFS tree
> landed in Linus's tree for this to get noticed. Fortify was in -next
> for a while...

There was a last-minute rebase of that tree. I don't see anything
relevant there. The code in question has been the same for ages. But I
most be overlooking something.... I guess it could be interesting to
bisect to figure out when the warning started.

--b.