On 4/25/24 01:49, Kees Cook wrote: > On Wed, Mar 20, 2024 at 11:48:52PM +0100, Helge Deller wrote: >> On 3/20/24 23:35, Justin Stitt wrote: >>> Hi, >>> >>> On Wed, Mar 20, 2024 at 12:56=E2=80=AFAM Helge Deller = wrote: >>>> >>>> On 3/19/24 00:46, Justin Stitt wrote: >>>>> strncpy() is deprecated for use on NUL-terminated destination string= s >>>>> [1] and as such we should prefer more robust and less ambiguous stri= ng >>>>> interfaces. >>>>> >>>>> Let's use the new 2-argument strscpy() which guarantees NUL-terminat= ion >>>>> on the destination buffer while also simplifying the syntax. Note th= at >>>>> strscpy() will not NUL-pad the destination buffer like strncpy() doe= s. >>>>> >>>>> However, the NUL-padding behavior of strncpy() is not required since >>>>> fbdev is already NUL-allocated from au1200fb_drv_probe() -> >>>>> frameuffer_alloc(), rendering any additional NUL-padding redundant. >>>>> | p =3D kzalloc(fb_info_size + size, GFP_KERNEL); >>>>> >>>>> Link: https://www.kernel.org/doc/html/latest/process/deprecated.html= #strncpy-on-nul-terminated-strings [1] >>>>> Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9= .en.html [2] >>>>> Link: https://github.com/KSPP/linux/issues/90 >>>>> Cc: linux-hardening@vger.kernel.org >>>>> Signed-off-by: Justin Stitt >>>>> --- >>>>> Note: build-tested only. >>>>> >>>>> Found with: $ rg "strncpy\(" >>>>> --- >>>>> drivers/video/fbdev/au1200fb.c | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/video/fbdev/au1200fb.c b/drivers/video/fbdev/au= 1200fb.c >>>>> index 6f20efc663d7..e718fea63662 100644 >>>>> --- a/drivers/video/fbdev/au1200fb.c >>>>> +++ b/drivers/video/fbdev/au1200fb.c >>>>> @@ -1557,7 +1557,7 @@ static int au1200fb_init_fbinfo(struct au1200f= b_device *fbdev) >>>>> return ret; >>>>> } >>>>> >>>>> - strncpy(fbi->fix.id, "AU1200", sizeof(fbi->fix.id)); >>>>> + strscpy(fbi->fix.id, "AU1200"); >>>> >>>> I wonder if you really build-tested this, as this driver is for the m= ips architecture... >>>> And I don't see a strscpy() function which takes just 2 arguments. >>>> But I might be wrong.... >>> >>> I did build successfully :thumbs_up: >>> >>> Commit e6584c3964f2f ("string: Allow 2-argument strscpy()") introduced >>> this new strscpy() form; it is present in string.h on Linus' tree. >> >> Interesting patch. >> Might give compile problems if patches like yours gets automatically >> picked up to stable series as long as Kees patch hasn't been backported= yet... >> Anyway, thanks for the pointer! >> I'll apply your patch in the next round for fbdev. > > Hi! I haven't seen this show up in -next yet. Have you had a chance to > pick it up? > > There are also these too: > > https://lore.kernel.org/all/20240320-strncpy-drivers-video-fbdev-fsl-diu= -fb-c-v1-1-3cd3c012fa8c@google.com/ > https://patchwork.kernel.org/project/linux-hardening/patch/20240320-strn= cpy-drivers-video-fbdev-uvesafb-c-v1-1-fd6af3766c80@google.com/ > https://patchwork.kernel.org/project/linux-hardening/patch/20240320-strn= cpy-drivers-video-hdmi-c-v1-1-f9a08168cdaf@google.com/ All 4 patches picked up into fbdev for-next git tree now. Thanks! Helge