linux-hardening - RE: [PATCH] net: txgbe: replace deprecated strncpy with strscpy

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date: Mon, 16 Oct 2023 11:05:29 +0800
From: Jiawen Wu <jiawenwu@...stnetic.com>
To: "'Andrew Lunn'" <andrew@...n.ch>,
	"'Justin Stitt'" <justinstitt@...gle.com>
Cc: "'Mengyuan Lou'" <mengyuanlou@...-swift.com>,
	"'David S. Miller'" <davem@...emloft.net>,
	"'Eric Dumazet'" <edumazet@...gle.com>,
	"'Jakub Kicinski'" <kuba@...nel.org>,
	"'Paolo Abeni'" <pabeni@...hat.com>,
	<netdev@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>,
	<linux-hardening@...r.kernel.org>
Subject: RE: [PATCH] net: txgbe: replace deprecated strncpy with strscpy

On Sunday, October 15, 2023 12:31 AM, Andrew Lunn wrote:
> On Thu, Oct 12, 2023 at 09:20:04PM +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.
> >
> > Based on usage part_str usage within txgbe_read_pba_string(), we expect
> > part_str to be NUL-terminated but not necessarily NUL-padded:
> > |       /* put a null character on the end of our string */
> > |       pba_num[10] = '\0';
> >
> > Interestingly, part_str is not used after txgbe_read_pba_string():
> > | ...
> > |	err = txgbe_read_pba_string(wx, part_str, TXGBE_PBANUM_LENGTH);
> > |	if (err)
> > |		strscpy(part_str, "Unknown", sizeof(part_str));
> > |
> > |	netif_info(wx, probe, netdev, "%pM\n", netdev->dev_addr);
> > |
> > |	return 0;
> > |
> > |err_remove_phy:
> > |	txgbe_remove_phy(txgbe);
> > |err_release_hw:
> > |	wx_clear_interrupt_scheme(wx);
> > |	wx_control_hw(wx, false);
> > |err_free_mac_table:
> > |	kfree(wx->mac_table);
> > |err_pci_release_regions:
> > |	pci_release_selected_regions(pdev,
> > |				     pci_select_bars(pdev, IORESOURCE_MEM));
> > |err_pci_disable_dev:
> > |	pci_disable_device(pdev);
> > |	return err;
> > |}
> > ... this means this strncpy (or now strscpy) is probably useless. For
> > now, let's make the swap to strscpy() as I am not sure if this is truly
> > dead code or not.
> 
> Hi Julian
> 
> I agree, this looks like dead code.
> 
> Jiawen, please could you submit a patch cleaning this up. Either swap
> to strscpy() and make use of the string, or delete it all.

Ah, I think I missed a line to print this string. I'm going to add it.