Messages in this thread | | | Date | Thu, 23 Jan 2020 10:13:13 +0300 | From | Dan Carpenter <> | Subject | Re: KASAN: slab-out-of-bounds Read in __nla_put_nohdr |
| |
On Wed, Jan 22, 2020 at 12:33:17PM -0800, Eric Dumazet wrote: > > > On 1/22/20 12:27 PM, Cong Wang wrote: > > On Tue, Jan 21, 2020 at 11:55 AM Eric Dumazet <eric.dumazet@gmail.com> wrote: > >> em_nbyte_change() sets > >> em->datalen = sizeof(*nbyte) + nbyte->len; > >> > >> But later tcf_em_validate() overwrites em->datalen with the user provide value (em->datalen = data_len; ) > >> which can be bigger than the allocated (kmemdup) space in em_nbyte_change() > >> > >> Should net/sched/em_nbyte.c() provide a dump() handler to avoid this issue ? > > > > I think for those who implement ->change() we should leave > > ->datalen untouched to respect their choices. I don't see why > > we have to set it twice. > > > > > > Agreed, but we need to audit them to make sure all of them are setting ->datalen >
Smatch provides a way to do this sort of search:
$ smdb where tcf_ematch datalen net/sched/ematch.c | tcf_em_validate | (struct tcf_ematch)->datalen | 0-65519 net/sched/ematch.c | tcf_em_tree_validate | (struct tcf_ematch)->datalen | 0 net/sched/em_ipt.c | em_ipt_change | (struct tcf_ematch)->datalen | 16-131080 net/sched/em_meta.c | em_meta_change | (struct tcf_ematch)->datalen | 48 net/sched/em_text.c | em_text_change | (struct tcf_ematch)->datalen | 16 net/sched/em_canid.c | em_canid_change | (struct tcf_ematch)->datalen | 276-4268 net/sched/em_ipset.c | em_ipset_change | (struct tcf_ematch)->datalen | 4 net/sched/em_nbyte.c | em_nbyte_change | (struct tcf_ematch)->datalen | 4-4099
It is imperfect... The main drawback is that it's based on my allmodconfig which might not include every function. But always there are other bugs to be discovered.
regards, dan carpenter
| |