Messages in this thread | | | From | Justin Stitt <> | Date | Mon, 16 Oct 2023 15:20:39 -0700 | Subject | Re: [PATCH] hamradio: replace deprecated strncpy with strscpy |
| |
On Mon, Oct 16, 2023 at 3:18 PM Hugh Blemings <hugh@blemings.org> wrote: > > > > On 17/10/23 04:03, Kees Cook wrote: > > > > > > On October 16, 2023 10:01:20 AM PDT, Kees Cook <kees@kernel.org> wrote: > >> > >> > >> On October 15, 2023 10:47:53 PM PDT, Dan Carpenter <dan.carpenter@linaro.org> wrote: > >>> On Sun, Oct 15, 2023 at 05:06:19PM +0200, Simon Horman wrote: > >>>> On Thu, Oct 12, 2023 at 09:33:32PM +0000, Justin Stitt wrote: > >>>>> strncpy() is deprecated for use on NUL-terminated destination strings > >>>>> [1] and as such we should prefer more robust and less ambiguous string > >>>>> interfaces. > >>>>> > >>>>> We expect both hi.data.modename and hi.data.drivername to be > >>>>> NUL-terminated but not necessarily NUL-padded which is evident by its > >>>>> usage with sprintf: > >>>>> | sprintf(hi.data.modename, "%sclk,%smodem,fclk=%d,bps=%d%s", > >>>>> | bc->cfg.intclk ? "int" : "ext", > >>>>> | bc->cfg.extmodem ? "ext" : "int", bc->cfg.fclk, bc->cfg.bps, > >>>>> | bc->cfg.loopback ? ",loopback" : ""); > >>>>> > >>>>> Note that this data is copied out to userspace with: > >>>>> | if (copy_to_user(data, &hi, sizeof(hi))) > >>>>> ... however, the data was also copied FROM the user here: > >>>>> | if (copy_from_user(&hi, data, sizeof(hi))) > >>>> > >>>> Thanks Justin, > >>>> > >>>> I see that too. > >>>> > >>>> Perhaps I am off the mark here, and perhaps it's out of scope for this > >>>> patch, but I do think it would be nicer if the kernel only sent > >>>> intended data to user-space, even if any unintended payload came > >>>> from user-space. > >>>> > >>> > >>> It's kind of normal to pass user space data back to itself. We > >>> generally only worry about info leaks. > >> > >> True but since this used to zero the rest of the buffet, let's just keep that behavior and use strscpy_pad(). > > > > I'm calling all byte arrays a "buffet" from now on. ;) > > > A tasteful change to the sauce I think. ;)
Just perfect that this is happening on a **ham**radio driver.
| |