Re: [PATCH 1/1 net-next] net: dsa: replace count*size kmalloc by kmalloc_array

[Fabian Frederick]

> On 14 November 2014 at 19:47 Joe Perches <joe@xxxxxxxxxxx> wrote:
>
>
> On Fri, 2014-11-14 at 19:36 +0100, Fabian Frederick wrote:
> > kmalloc_array manages count*sizeof overflow.
>
> Fundamentally correct, but is this necessary or useful?
> sizeof(s8) isn't often going to be anything other than 1.
Absolutely, I thought it was a struct :)

There must be a reason for so many cases though ...

Regards,
Fabian

>
> Would the kernel even work without that assumption?
>
>
> > diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
> []
> > @@ -526,7 +526,8 @@ static int dsa_of_setup_routing_table(struct
> > dsa_platform_data *pd,
> >Â
> >Â Â Â/* First time routing table allocation */
> >Â Â Âif (!cd->rtable) {
> > -Â Â Â Â Â Âcd->rtable = kmalloc(pd->nr_chips * sizeof(s8), GFP_KERNEL);
> > +Â Â Â Â Â Âcd->rtable = kmalloc_array(pd->nr_chips, sizeof(s8),
> > +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â GFP_KERNEL);
> >Â Â Â Â Â Â Âif (!cd->rtable)
> >Â Â Â Â Â Â Â Â Â Â Âreturn -ENOMEM;
> >Â
>
> Maybe all of these could be simplified
>
> $ git grep -E "\*\s*sizeof\s*\(\s*[us]8\s*\)"
> arch/arm/common/edma.c:Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â(edma_cc->num_tc +
> 1) * sizeof(s8),
> drivers/acpi/utils.c:Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â(element->buffer.length *
> sizeof(u8));
> drivers/acpi/utils.c:Â Â Â Â Â Â Â Â Â Â Â Â Â Âtail += element->buffer.length
> * sizeof(u8);
> drivers/char/tpm/tpm_i2c_stm_st33.c:Â Â Â Â kmalloc(TPM_BUFSIZE * sizeof(u8),
> GFP_KERNEL);
> drivers/char/tpm/tpm_i2c_stm_st33.c:Â Â Â Â kmalloc(TPM_BUFSIZE * sizeof(u8),
> GFP_KERNEL);
> drivers/gpu/drm/r128/r128_state.c:Â Â Â mask_size = depth->n * sizeof(u8);
> drivers/gpu/drm/r128/r128_state.c:Â Â Â Â Â Â Â mask_size = depth->n *
> sizeof(u8);
> drivers/iio/common/st_sensors/st_sensors_spi.c: memcpy(data, tb->rx_buf,
> len*sizeof(u8));
> drivers/infiniband/hw/amso1100/c2_mq.h: u8 pad[64 - sizeof(u16) - 2 *
> sizeof(u8) - sizeof(u32) - sizeof(u16)];
> drivers/input/tablet/aiptek.c:Â const int sizeof_buf = 3 * sizeof(u8);
> drivers/input/tablet/aiptek.c:Â const int sizeof_buf = 3 * sizeof(u8);
> drivers/md/dm-crypt.c:Â memset(&cc->key, 0, cc->key_size * sizeof(u8));
> drivers/md/dm-crypt.c:Â cc = kzalloc(sizeof(*cc) + key_size * sizeof(u8),
> GFP_KERNEL);
> drivers/media/dvb-frontends/dib7000p.c: tx = kzalloc(2*sizeof(u8),
> GFP_KERNEL);
> drivers/media/dvb-frontends/dib7000p.c: rx = kzalloc(2*sizeof(u8),
> GFP_KERNEL);
> drivers/media/dvb-frontends/dib8000.c:Â client.i2c_write_buffer = kzalloc(4 *
> sizeof(u8), GFP_KERNEL);
> drivers/media/dvb-frontends/dib8000.c:Â client.i2c_read_buffer = kzalloc(4 *
> sizeof(u8), GFP_KERNEL);
> drivers/media/dvb-frontends/dib9000.c:Â client.i2c_write_buffer = kzalloc(4 *
> sizeof(u8), GFP_KERNEL);
> drivers/media/dvb-frontends/dib9000.c:Â client.i2c_read_buffer = kzalloc(4 *
> sizeof(u8), GFP_KERNEL);
> drivers/media/pci/ttpci/av7110_ipack.c: if (!(p->buf =
> vmalloc(size*sizeof(u8)))) {
> drivers/mtd/inftlmount.c:Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âs->nb_blocks *
> sizeof(u8));
> drivers/net/wireless/ath/ath10k/htt.h:Â Â*Â b) num_chars * sizeof(u8) aligned
> to 4bytes */
> drivers/net/wireless/b43/ppr.c: BUILD_BUG_ON(sizeof(struct b43_ppr) !=
> B43_PPR_RATES_NUM * sizeof(u8));
> drivers/net/wireless/iwlwifi/pcie/trans.c:Â Â Â Â Â Â Â Â Â Â
> Âtrans_pcie->n_no_reclaim_cmds * sizeof(u8));
> drivers/net/wireless/rtlwifi/efuse.c:Â Âmemset(data, 0xff, PGPKT_DATA_SIZE *
> sizeof(u8));
> drivers/net/wireless/rtlwifi/efuse.c:Â Âmemset(tmpdata, 0xff, PGPKT_DATA_SIZE
> * sizeof(u8));
> drivers/net/wireless/rtlwifi/efuse.c:Â Âu8 originaldata[8 * sizeof(u8)];
> drivers/net/wireless/rtlwifi/efuse.c:Â Âu8 originaldata[8 * sizeof(u8)];
> drivers/net/wireless/rtlwifi/efuse.c:     Âmemset(originaldata, 0xff, 8
> * sizeof(u8));
> drivers/net/wireless/rtlwifi/efuse.c: Âmemset(target_pkt.data, 0xFF, 8 *
> sizeof(u8));
> drivers/power/ds2781_battery.c: ret = w1_ds2781_read(dev_info, val,
> DS2781_VOLT_MSB, 2 * sizeof(u8));
> drivers/power/ds2781_battery.c: ret = w1_ds2781_read(dev_info, val,
> DS2781_TEMP_MSB, 2 * sizeof(u8));
> drivers/rtc/rtc-pcf2123.c:Â Â Â ret = spi_write(spi, txbuf, 2 * sizeof(u8));
> drivers/rtc/rtc-pcf2123.c:Â Â Â ret = spi_write(spi, txbuf, 2 * sizeof(u8));
> drivers/rtc/rtc-pcf2123.c:Â Â Â ret = spi_write_then_read(spi, txbuf, 1 *
> sizeof(u8),
> drivers/rtc/rtc-pcf2123.c:Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â rxbuf, 2 *
> sizeof(u8));
> drivers/thermal/x86_pkg_temp_thermal.c:Â Â Â Â Â Â Â Â Â(max_phy_id+1) *
> sizeof(u8), GFP_ATOMIC);
> fs/compat_ioctl.c:Â Â Â if (__copy_in_user(&tdata->read_write,
> &udata->read_write, 2 * sizeof(u8)))
> net/dsa/dsa.c:Â Â Â Â Â cd->rtable = kmalloc(pd->nr_chips * sizeof(s8),
> GFP_KERNEL);
> net/dsa/dsa.c:Â Â Â Â Â memset(cd->rtable, -1, pd->nr_chips * sizeof(s8));
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/