| Message ID | 20200311083745.17328-7-tiwai@suse.de |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | net: Use scnprintf() for avoiding potential buffer overflow | expand |
Hi Takashi, Fix looks ok, but could you please fix the alignment of the subsequent lines as well? 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@solarflare.com> > Cc: Edward Cree <ecree@solarflare.com> > Cc: Martin Habets <mhabets@solarflare.com> > Signed-off-by: Takashi Iwai <tiwai@suse.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); > >
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@solarflare.com> > > Cc: Edward Cree <ecree@solarflare.com> > > Cc: Martin Habets <mhabets@solarflare.com> > > Signed-off-by: Takashi Iwai <tiwai@suse.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); > > > > >
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);
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@solarflare.com> Cc: Edward Cree <ecree@solarflare.com> Cc: Martin Habets <mhabets@solarflare.com> Signed-off-by: Takashi Iwai <tiwai@suse.de> --- drivers/net/ethernet/sfc/mcdi.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)