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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 7 Feb 2022 09:08:33 -0800 From: Yonghong Song <yhs@...com> To: Lorenzo Bianconi <lorenzo.bianconi@...hat.com> CC: Lorenzo Bianconi <lorenzo@...nel.org>, <bpf@...r.kernel.org>, <ast@...nel.org>, <daniel@...earbox.net>, <brouer@...hat.com>, <toke@...hat.com>, <andrii@...nel.org>, <netdev@...r.kernel.org> Subject: Re: [PATCH bpf-next] selftest/bpf: check invalid length in test_xdp_update_frags On 2/4/22 11:08 AM, Lorenzo Bianconi wrote: >>> >> >> [...] >> >>>>> >>>>> In kernel, the nr_frags checking is against MAX_SKB_FRAGS, >>>>> but if /proc/sys/net/core/max_skb_flags is 2 or more less >>>>> than MAX_SKB_FRAGS, the test won't fail, right? >>>> >>>> yes, you are right. Should we use the same definition used in >>>> include/linux/skbuff.h instead? Something like: >>>> >>>> if (65536 / page_size + 1 < 16) >>>> max_skb_flags = 16; >>>> else >>>> max_skb_flags = 65536/page_size + 1; >>> >>> The maximum packet size limit 64KB won't change anytime soon. >>> So the above should work. Some comments to explain why using >>> the above formula will be good. >> >> ack, I will do in v2. > > I can see there is a on-going discussion here [0] about increasing > MAX_SKB_FRAGS. I guess we can put on-hold this patch and see how > MAX_SKB_FRAGS will be changed. Thanks for the link. The new patch is going to increase MAX_SKB_FRAGS and it is possible that will be changed again (maybe under some config options). The default value for /proc/sys/net/core/max_skb_flags is MAX_SKB_FRAGS and I suspect anybody is bothering to change it. So your patch is okay to me. Maybe change a little bit -ENOMEM error message. current, ASSERT_EQ(err, -ENOMEM, "unsupported buffer size"); to ASSERT_EQ(err, -ENOMEM, "unsupported buffer size, possible non-default /proc/sys/net/core/max_skb_flags?"); > > Regards, > Lorenzo > > [0] https://lore.kernel.org/all/202202031315.B425Ipe8-lkp@intel.com/t/#ma1b2c7e71fe9bc69e24642a62dadf32fda7d5f03 > >> >> Regards, >> Lorenzo >> >>> >>>> >>>> Regards, >>>> Lorenzo >>>> >>>>> >>>>>> + >>>>>> + num = fscanf(f, "%d", &max_skb_frags); >>>>>> + fclose(f); >>>>>> + >>>>>> + if (!ASSERT_EQ(num, 1, "max_skb_frags read failed")) >>>>>> + goto out; >>>>>> + >>>>>> + /* xdp_buff linear area size is always set to 4096 in the >>>>>> + * bpf_prog_test_run_xdp routine. >>>>>> + */ >>>>>> + buf_size = 4096 + (max_skb_frags + 1) * sysconf(_SC_PAGE_SIZE); >>>>>> + buf = malloc(buf_size); >>>>>> + if (!ASSERT_OK_PTR(buf, "alloc buf")) >>>>>> + goto out; >>>>>> + >>>>>> + memset(buf, 0, buf_size); >>>>>> + offset = (__u32 *)buf; >>>>>> + *offset = 16; >>>>>> + buf[*offset] = 0xaa; >>>>>> + buf[*offset + 15] = 0xaa; >>>>>> + >>>>>> + topts.data_in = buf; >>>>>> + topts.data_out = buf; >>>>>> + topts.data_size_in = buf_size; >>>>>> + topts.data_size_out = buf_size; >>>>>> + >>>>>> + err = bpf_prog_test_run_opts(prog_fd, &topts); >>>>>> + ASSERT_EQ(err, -ENOMEM, "unsupported buffer size"); >>>>>> + free(buf); >>>>>> out: >>>>>> bpf_object__close(obj); >>>>>> } >>>>> >>> > >
Powered by blists - more mailing lists