2021-05-06 15:04:45

by Pavel Skripkin

[permalink] [raw]
Subject: Re: [syzbot] WARNING in __vmalloc_node_range

On Thu, 6 May 2021 17:57:22 +0300
Dan Carpenter <[email protected]> wrote:

> On Thu, May 06, 2021 at 04:22:10PM +0200, Uladzislau Rezki wrote:
> > Seems like vmalloc() is called with zero size passed:
> >
> > <snip>
> > void *__vmalloc_node_range(unsigned long size, unsigned long align,
> > unsigned long start, unsigned long end,
> > gfp_t gfp_mask, pgprot_t prot, unsigned long vm_flags, int node,
> > const void *caller)
> > {
> > struct vm_struct *area;
> > void *addr;
> > unsigned long real_size = size;
> > unsigned long real_align = align;
> > unsigned int shift = PAGE_SHIFT;
> >
> > 2873 if (WARN_ON_ONCE(!size))
> > return NULL;
> > <snip>
> >
> > from the dvb_dmx_init() driver:
> >
> > <snip>
> > int dvb_dmx_init(struct dvb_demux *dvbdemux)
> > {
> > int i;
> > struct dmx_demux *dmx = &dvbdemux->dmx;
> >
> > dvbdemux->cnt_storage = NULL;
> > dvbdemux->users = 0;
> > 1251 dvbdemux->filter = vmalloc(array_size(sizeof(struct
> > dvb_demux_filter), <snip>
> > dvbdemux->filternum));
>
> Indeed.
>
> It is a mystery because array_size() should never return less than
> sizeof(struct dvb_demux_filter). That's the whole point of the
> array_size() function is that it returns ULONG_MAX if there is an
> integer overflow.
>
> regards,
> dan carpenter
>
>
>

Hi!

I've already sent the patch:
https://patchwork.linuxtv.org/project/linux-media/patch/[email protected]/


With regards,
Pavel Skripkin


2021-05-07 09:08:42

by Dan Carpenter

[permalink] [raw]
Subject: Re: [syzbot] WARNING in __vmalloc_node_range

On Thu, May 06, 2021 at 06:00:53PM +0300, Pavel Skripkin wrote:
>
> Hi!
>
> I've already sent the patch:
> https://patchwork.linuxtv.org/project/linux-media/patch/[email protected]/
>

Please, always add a Fixes tag.

Fixes: 4d43e13f723e ("V4L/DVB (4643): Multi-input patch for DVB-USB device")

regards,
dan carpenter

2021-05-07 16:41:26

by Pavel Skripkin

[permalink] [raw]
Subject: Re: [syzbot] WARNING in __vmalloc_node_range

On Fri, 7 May 2021 11:04:36 +0300
Dan Carpenter <[email protected]> wrote:

> On Thu, May 06, 2021 at 06:00:53PM +0300, Pavel Skripkin wrote:
> >
> > Hi!
> >
> > I've already sent the patch:
> > https://patchwork.linuxtv.org/project/linux-media/patch/[email protected]/
> >
>
> Please, always add a Fixes tag.
>
> Fixes: 4d43e13f723e ("V4L/DVB (4643): Multi-input patch for DVB-USB
> device")
>
> regards,
> dan carpenter
>

oh..., that's one thing I always forget about. Thanks for pointing it
out, I'll send v2 soon


With regards,
Pavel Skripkin

2021-05-07 17:49:53

by Uladzislau Rezki

[permalink] [raw]
Subject: Re: [syzbot] WARNING in __vmalloc_node_range

Hello, Pavel.

Also in the commit message i see a type.

<snip>
syzbot reported WARNING in vmalloc. The problem
was in sizo size passed to vmalloc.
<snip>

Should it be "...zero size passed to vmalloc"?

--
Vlad Rezki


On Fri, May 7, 2021 at 2:29 PM Pavel Skripkin <[email protected]> wrote:
>
> On Fri, 7 May 2021 11:04:36 +0300
> Dan Carpenter <[email protected]> wrote:
>
> > On Thu, May 06, 2021 at 06:00:53PM +0300, Pavel Skripkin wrote:
> > >
> > > Hi!
> > >
> > > I've already sent the patch:
> > > https://patchwork.linuxtv.org/project/linux-media/patch/[email protected]/
> > >
> >
> > Please, always add a Fixes tag.
> >
> > Fixes: 4d43e13f723e ("V4L/DVB (4643): Multi-input patch for DVB-USB
> > device")
> >
> > regards,
> > dan carpenter
> >
>
> oh..., that's one thing I always forget about. Thanks for pointing it
> out, I'll send v2 soon
>
>
> With regards,
> Pavel Skripkin



--
Uladzislau Rezki

2021-05-08 12:48:05

by Dan Carpenter

[permalink] [raw]
Subject: Re: [syzbot] WARNING in __vmalloc_node_range

I wrote a Smatch check to see if there were more of these and here are
the other issues that it found. (I will expant this check to more types
on Monday).

drivers/media/usb/dvb-usb-v2/lmedm04.c:1196 (null)() warn: element count is wrong 'lme2510_props.num_adapters=0' vs 'lme2510_props.adapter=2'
drivers/media/usb/dvb-usb-v2/af9035.c:1997 (null)() warn: element count is wrong 'af9035_props.num_adapters=0' vs 'af9035_props.adapter=2'
drivers/media/usb/dvb-usb-v2/af9035.c:2043 (null)() warn: element count is wrong 'it930x_props.num_adapters=0' vs 'it930x_props.adapter=2'
drivers/media/usb/dvb-usb-v2/af9015.c:1409 (null)() warn: element count is wrong 'af9015_props.num_adapters=0' vs 'af9015_props.adapter=2'
drivers/media/usb/dvb-usb/dtt200u.c:384 (null)() warn: element count is wrong 'wt220u_miglia_properties.num_adapters=1' vs 'wt220u_miglia_properties.adapter=0'

As far as I can see these are initialized in dvb_usb_adapter_init()
where the loop is:

for (n = 0; n < d->props.num_adapters; n++) {

So it looks like all of these are genuine bugs. But I'm not a subsystem
expert and can't test them. These line numbers are from linux-next.

Btw, here are the other element/count pairings I was able to find which
I'm going to test on Monday.

ath5k_gain_opt, go_steps_count, go_step
atomisp_camera_caps, sensor_num, sensor
brcmf_rssi_event_le, rssi_level_num, rssi_levels
catpt_stream_template, num_entries, entries
dvb_usb_device_properties, num_adapters, adapter
dvb_usb_device_properties, num_device_descs, devices
go7007_board_info, num_inputs, inputs
hda_input_mux, num_items, items
idt_89hpes_cfg, port_cnt, ports
mipi_phy_device_desc, num_regmaps, regmap_names
mtk_thermal_data, need_switch_bank, bank_data
mwifiex_sdio_card_reg, func1_spec_reg_num, func1_spec_reg_table
nft_chain_type, hook_mask, hooks
PWR_DFY_Section, dfy_size, dfy_data
rkisp1_cif_isp_afc_config, num_afm_win, afm_win
scarlett_device_info, num_controls, controls
snd_soc_acpi_codecs, num_codecs, codecs
timb_dma_platform_data, nr_channels, channels
uniphier_u3hsphy_soc_data, nparams, param
uniphier_u3ssphy_soc_data, nparams, param
venus_resources, vcodec_clks_num, vcodec_pmdomains

regards,
dan carpenter

2021-05-11 07:11:49

by Dan Carpenter

[permalink] [raw]
Subject: Re: [syzbot] WARNING in __vmalloc_node_range

On Sat, May 08, 2021 at 03:46:30PM +0300, Dan Carpenter wrote:
> I wrote a Smatch check to see if there were more of these and here are
> the other issues that it found. (I will expant this check to more types
> on Monday).
>
> drivers/media/usb/dvb-usb-v2/lmedm04.c:1196 (null)() warn: element count is wrong 'lme2510_props.num_adapters=0' vs 'lme2510_props.adapter=2'

This one is fine, but could do with some cleaning up.

> drivers/media/usb/dvb-usb-v2/af9035.c:1997 (null)() warn: element count is wrong 'af9035_props.num_adapters=0' vs 'af9035_props.adapter=2'
> drivers/media/usb/dvb-usb-v2/af9035.c:2043 (null)() warn: element count is wrong 'it930x_props.num_adapters=0' vs 'it930x_props.adapter=2'
> drivers/media/usb/dvb-usb-v2/af9015.c:1409 (null)() warn: element count is wrong 'af9015_props.num_adapters=0' vs 'af9015_props.adapter=2'

These are false positives because they use the .get_adapter_count()
function instead of setting num_adapters.

> drivers/media/usb/dvb-usb/dtt200u.c:384 (null)() warn: element count is wrong 'wt220u_miglia_properties.num_adapters=1' vs 'wt220u_miglia_properties.adapter=0'

I'm not sure what's going on with this one... It still looks buggy to
me.

I did re-run Smatch with more elem/count pairs checked and there were
no bugs found. A bunch of drivers think you need to add a zeroed
element at the end of the .devices[] array so someone could delete that
if they wanted.

drivers/media/usb/dvb-usb/vp702x.c:374 (null)() warn: element count is wrong 'vp702x_properties.num_device_descs=1' vs 'vp702x_properties.devices=2'
drivers/media/usb/dvb-usb/vp7045.c:184 (null)() warn: element count is wrong 'vp7045_properties.num_device_descs=2' vs 'vp7045_properties.devices=3'
drivers/media/usb/dvb-usb/cinergyT2-core.c:206 (null)() warn: element count is wrong 'cinergyt2_properties.num_device_descs=1' vs 'cinergyt2_properties.devices=2'
drivers/media/usb/dvb-usb/digitv.c:300 (null)() warn: element count is wrong 'digitv_properties.num_device_descs=1' vs 'digitv_properties.devices=2'
drivers/media/usb/dvb-usb/dibusb-mc.c:48 (null)() warn: element count is wrong 'dibusb_mc_properties.num_device_descs=8' vs 'dibusb_mc_properties.devices=9'
drivers/media/usb/dvb-usb/pctv452e.c:963 (null)() warn: element count is wrong 'pctv452e_properties.num_device_descs=1' vs 'pctv452e_properties.devices=2'
drivers/media/usb/dvb-usb/pctv452e.c:1015 (null)() warn: element count is wrong 'tt_connect_s2_3600_properties.num_device_descs=2' vs 'tt_connect_s2_3600_properties.devices=3'
drivers/media/usb/dvb-usb/gp8psk.c:324 (null)() warn: element count is wrong 'gp8psk_properties.num_device_descs=4' vs 'gp8psk_properties.devices=5'
drivers/media/usb/dvb-usb/nova-t-usb2.c:168 (null)() warn: element count is wrong 'nova_t_properties.num_device_descs=1' vs 'nova_t_properties.devices=2'
drivers/media/usb/dvb-usb/dibusb-mb.c:267 (null)() warn: element count is wrong 'dibusb1_1_an2235_properties.num_device_descs=2' vs 'dibusb1_1_an2235_properties.devices=3'
drivers/media/usb/dvb-usb/dibusb-mb.c:335 (null)() warn: element count is wrong 'dibusb2_0b_properties.num_device_descs=2' vs 'dibusb2_0b_properties.devices=3'
drivers/media/usb/dvb-usb/dibusb-mb.c:398 (null)() warn: element count is wrong 'artec_t1_usb2_properties.num_device_descs=1' vs 'artec_t1_usb2_properties.devices=2'
drivers/media/usb/dvb-usb/af9005.c:1015 (null)() warn: element count is wrong 'af9005_properties.num_device_descs=3' vs 'af9005_properties.devices=4'
drivers/media/usb/dvb-usb/dtt200u.c:176 (null)() warn: element count is wrong 'dtt200u_properties.num_device_descs=1' vs 'dtt200u_properties.devices=2'
drivers/media/usb/dvb-usb/dtt200u.c:228 (null)() warn: element count is wrong 'wt220u_properties.num_device_descs=1' vs 'wt220u_properties.devices=2'
drivers/media/usb/dvb-usb/dtt200u.c:280 (null)() warn: element count is wrong 'wt220u_fc_properties.num_device_descs=1' vs 'wt220u_fc_properties.devices=2'
drivers/media/usb/dvb-usb/dtt200u.c:332 (null)() warn: element count is wrong 'wt220u_zl0353_properties.num_device_descs=1' vs 'wt220u_zl0353_properties.devices=2'
drivers/media/usb/dvb-usb/dtt200u.c:384 (null)() warn: element count is wrong 'wt220u_miglia_properties.num_device_descs=1' vs 'wt220u_miglia_properties.devices=2'
drivers/media/usb/dvb-usb/az6027.c:1096 (null)() warn: element count is wrong 'az6027_properties.num_device_descs=8' vs 'az6027_properties.devices=9'

regards,
dan carpenter