netdev - Re: [PATCH 6/7] sfc: Use scnprintf() for avoiding potential buffer overflow

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <s5hv9n9g5w9.wl-tiwai@suse.de>
Date:   Thu, 12 Mar 2020 12:38:14 +0100
From:   Takashi Iwai <tiwai@...e.de>
To:     Martin Habets <mhabets@...arflare.com>
Cc:     Takashi Iwai <tiwai@...e.de>, <netdev@...r.kernel.org>,
        "David S . Miller" <davem@...emloft.net>,
        Solarflare linux maintainers <linux-net-drivers@...arflare.com>,
        Edward Cree <ecree@...arflare.com>
Subject: Re: [PATCH 6/7] sfc: Use scnprintf() for avoiding potential buffer overflow

On Thu, 12 Mar 2020 10:53:05 +0100,
Martin Habets wrote:
> 
> Hi Takashi,
> 
> Fix looks ok, but could you please fix the alignment of the subsequent lines as well?

Yes, I'll respin with that in v2, as other people also asked for it.


thanks,

Takashi

> 
> Thanks,
> Martin
> 
> On 11/03/2020 08:37, Takashi Iwai wrote:
> > Since snprintf() returns the would-be-output size instead of the
> > actual output size, the succeeding calls may go beyond the given
> > buffer limit.  Fix it by replacing with scnprintf().
> > 
> > Cc: Solarflare linux maintainers <linux-net-drivers@...arflare.com>
> > Cc: Edward Cree <ecree@...arflare.com>
> > Cc: Martin Habets <mhabets@...arflare.com>
> > Signed-off-by: Takashi Iwai <tiwai@...e.de>
> > ---
> >  drivers/net/ethernet/sfc/mcdi.c | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/sfc/mcdi.c b/drivers/net/ethernet/sfc/mcdi.c
> > index 2713300343c7..ac978e24644f 100644
> > --- a/drivers/net/ethernet/sfc/mcdi.c
> > +++ b/drivers/net/ethernet/sfc/mcdi.c
> > @@ -212,11 +212,11 @@ static void efx_mcdi_send_request(struct efx_nic *efx, unsigned cmd,
> >  		 * progress on a NIC at any one time.  So no need for locking.
> >  		 */
> >  		for (i = 0; i < hdr_len / 4 && bytes < PAGE_SIZE; i++)
> > -			bytes += snprintf(buf + bytes, PAGE_SIZE - bytes,
> > +			bytes += scnprintf(buf + bytes, PAGE_SIZE - bytes,
> >  					  " %08x", le32_to_cpu(hdr[i].u32[0]));
> >  
> >  		for (i = 0; i < inlen / 4 && bytes < PAGE_SIZE; i++)
> > -			bytes += snprintf(buf + bytes, PAGE_SIZE - bytes,
> > +			bytes += scnprintf(buf + bytes, PAGE_SIZE - bytes,
> >  					  " %08x", le32_to_cpu(inbuf[i].u32[0]));
> >  
> >  		netif_info(efx, hw, efx->net_dev, "MCDI RPC REQ:%s\n", buf);
> > @@ -302,14 +302,14 @@ static void efx_mcdi_read_response_header(struct efx_nic *efx)
> >  		 */
> >  		for (i = 0; i < hdr_len && bytes < PAGE_SIZE; i++) {
> >  			efx->type->mcdi_read_response(efx, &hdr, (i * 4), 4);
> > -			bytes += snprintf(buf + bytes, PAGE_SIZE - bytes,
> > +			bytes += scnprintf(buf + bytes, PAGE_SIZE - bytes,
> >  					  " %08x", le32_to_cpu(hdr.u32[0]));
> >  		}
> >  
> >  		for (i = 0; i < data_len && bytes < PAGE_SIZE; i++) {
> >  			efx->type->mcdi_read_response(efx, &hdr,
> >  					mcdi->resp_hdr_len + (i * 4), 4);
> > -			bytes += snprintf(buf + bytes, PAGE_SIZE - bytes,
> > +			bytes += scnprintf(buf + bytes, PAGE_SIZE - bytes,
> >  					  " %08x", le32_to_cpu(hdr.u32[0]));
> >  		}
> >  
> > @@ -1417,7 +1417,7 @@ void efx_mcdi_print_fwver(struct efx_nic *efx, char *buf, size_t len)
> >  	}
> >  
> >  	ver_words = (__le16 *)MCDI_PTR(outbuf, GET_VERSION_OUT_VERSION);
> > -	offset = snprintf(buf, len, "%u.%u.%u.%u",
> > +	offset = scnprintf(buf, len, "%u.%u.%u.%u",
> >  			  le16_to_cpu(ver_words[0]), le16_to_cpu(ver_words[1]),
> >  			  le16_to_cpu(ver_words[2]), le16_to_cpu(ver_words[3]));
> >  
> > @@ -1427,7 +1427,7 @@ void efx_mcdi_print_fwver(struct efx_nic *efx, char *buf, size_t len)
> >  	if (efx_nic_rev(efx) >= EFX_REV_HUNT_A0) {
> >  		struct efx_ef10_nic_data *nic_data = efx->nic_data;
> >  
> > -		offset += snprintf(buf + offset, len - offset, " rx%x tx%x",
> > +		offset += scnprintf(buf + offset, len - offset, " rx%x tx%x",
> >  				   nic_data->rx_dpcpu_fw_id,
> >  				   nic_data->tx_dpcpu_fw_id);
> >  
> > 
>