Message ID | YvfGY2qnl2YXrUgX@shikoro |
---|---|
State | New |
Headers | show |
Series | [PULL,REQUEST] i2c-for-5.20-part2 | expand |
On Sat, Aug 13, 2022 at 8:42 AM Wolfram Sang <wsa@kernel.org> wrote: > > * subsystem-wide move from strlcpy to strscpy Hmm. Looking around, we still do have a lot of those 'strlcpy()' users, but 99% of them don't use the return value. And since 'strlcpy()' is complete and utter garbage exactly *BECAUSE* of the bad return value (it returns the length of the source string - but an untrusted and possibly unterminated source string is often the *problem* in the first place), that would make it fairly easy to convert the rest. I see two possibilities: (a) fix strlcpy() by making it return 'void', and leave all those 99% of users alone, and convert the (few) cases that actually look at the return value to strscpy (b) auto-convert (with a coccinelle script) all the 'strlcpy()' users that don't care about the return value, and leave a few broken users of strlcpy around I think (b) is the simpler thing, but I have no idea how to write a coccinelle patch that basically does "if return value of strlcpy is not used, convert the strlcpy to a strscpy". I'd love to get rid of strlcpy() entirely, since it's such a horribly badly designed thing. Linus
The pull request you sent on Sat, 13 Aug 2022 17:42:27 +0200:
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git tags/i2c-for-5.20-part2
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/04734361953b8cc30d5552c1abe907ccfc8bc0fa
Thank you!
> (b) auto-convert (with a coccinelle script) all the 'strlcpy()' users > that don't care about the return value, and leave a few broken users > of strlcpy around > > I think (b) is the simpler thing, but I have no idea how to write a > coccinelle patch that basically does "if return value of strlcpy is > not used, convert the strlcpy to a strscpy". That shouldn't be hard. I'll give it a try tomorrow.
On Sun, Aug 14, 2022 at 02:33:40AM +0200, Wolfram Sang wrote: > > > (b) auto-convert (with a coccinelle script) all the 'strlcpy()' users > > that don't care about the return value, and leave a few broken users > > of strlcpy around > > > > I think (b) is the simpler thing, but I have no idea how to write a > > coccinelle patch that basically does "if return value of strlcpy is > > not used, convert the strlcpy to a strscpy". > > That shouldn't be hard. I'll give it a try tomorrow. So, I tried right away and it was really as easy as expected: === @@ @@ - strlcpy + strscpy (...); === The ';' at the end ensures that spatch only looks for complete statements not containing anything more than expressed. I pushed out a branch for the testbots now: git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/strlcpy Patch granularity might not be perfect currently, but for testing this should do. All the best, Wolfram
On Sun, 14 Aug 2022, Wolfram Sang wrote: > On Sun, Aug 14, 2022 at 02:33:40AM +0200, Wolfram Sang wrote: > > > > > (b) auto-convert (with a coccinelle script) all the 'strlcpy()' users > > > that don't care about the return value, and leave a few broken users > > > of strlcpy around > > > > > > I think (b) is the simpler thing, but I have no idea how to write a > > > coccinelle patch that basically does "if return value of strlcpy is > > > not used, convert the strlcpy to a strscpy". > > > > That shouldn't be hard. I'll give it a try tomorrow. > > So, I tried right away and it was really as easy as expected: > > === > @@ > @@ > - strlcpy > + strscpy > (...); > === > > The ';' at the end ensures that spatch only looks for complete > statements not containing anything more than expressed. This is correct. julia > > I pushed out a branch for the testbots now: > > git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/strlcpy > > Patch granularity might not be perfect currently, but for testing this > should do. > > All the best, > > Wolfram > >
> I pushed out a branch for the testbots now: > > git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/strlcpy Pushed again with patches for userspace programs in 'tools/' and 'arch/um/os-Linux' removed. That was too late yesterday to notice them.
> Pushed again with patches for userspace programs in 'tools/' and > 'arch/um/os-Linux' removed. That was too late yesterday to notice them. Got a 'success' message now from the buildbot for the above branch.