From: "zhudi (J)" <zhudi21@huawei•com>
To: Davide Caratti <dcaratti@redhat•com>,
"jhs@mojatatu•com" <jhs@mojatatu•com>,
"xiyou.wangcong@gmail•com" <xiyou.wangcong@gmail•com>
Cc: "davem@davemloft•net" <davem@davemloft•net>,
"kuba@kernel•org" <kuba@kernel•org>,
"netdev@vger•kernel.org" <netdev@vger•kernel.org>,
"Chenxiang (EulerOS)" <rose.chen@huawei•com>
Subject: 答复: [PATCH] net/sched: act_pedit: fix a NULL pointer deref in tcf_pedit_init
Date: Tue, 9 Mar 2021 11:24:33 +0000 [thread overview]
Message-ID: <672f06766f2d49ecbb573037b3cb445a@huawei.com> (raw)
In-Reply-To: <07afbd8d9a76f3c0f0a0eb01759118a0c9e966a3.camel@redhat.com>
>
> hello, thanks for the patch!
>
> On Tue, 2021-03-09 at 11:47 +0800, zhudi wrote:
> > From: Di Zhu <zhudi21@huawei•com>
> >
> > when we use syzkaller to fuzz-test our kernel, one NULL pointer
> dereference
> > BUG happened:
> >
> > Write of size 96 at addr 0000000000000010 by task syz-executor.0/22376
> >
> ========================================================== ========
> > BUG: unable to handle kernel NULL pointer dereference at
> 0000000000000010
> > PGD 80000001dc1a9067 P4D 80000001dc1a9067 PUD 1a32b5067 PMD 0
> > [...]
> > Call Trace
> > memcpy include/linux/string.h:345 [inline]
> > tcf_pedit_init+0x7b4/0xa10 net/sched/act_pedit.c:232
> > tcf_action_init_1+0x59b/0x730 net/sched/act_api.c:920
> > tcf_action_init+0x1ef/0x320 net/sched/act_api.c:975
> > tcf_action_add+0xd2/0x270 net/sched/act_api.c:1360
> > tc_ctl_action+0x267/0x290 net/sched/act_api.c:1412
> > [...]
> >
> > The root cause is that we use kmalloc() to allocate mem space for
> > keys without checking if the ksize is 0.
>
> actually Linux does this:
>
> 173 parm = nla_data(pattr);
> 174 if (!parm->nkeys) {
> 175 NL_SET_ERR_MSG_MOD(extack, "Pedit requires keys to be
> passed");
> 176 return -EINVAL;
> 177 }
> 178 ksize = parm->nkeys * sizeof(struct tc_pedit_key);
> 179 if (nla_len(pattr) < sizeof(*parm) + ksize) {
> 180 NL_SET_ERR_MSG_ATTR(extack, pattr, "Length of
> TCA_PEDIT_PARMS or TCA_PEDIT_PARMS_EX pedit attribute is invalid");
> 181 return -EINVAL;
> 182 }
>
> maybe it's not sufficient? If so, we can add something here. I'd prefer
> to disallow inserting pedit actions with p->tcfp_nkeys equal to zero,
> because they are going to trigger a WARN(1) in the traffic path (see
> tcf_pedit_act() at the bottom).
Yes, you are right. I didn't notice your code submission(commit-id is f67169fef8dbcc1a) in 2019
and the kernel we tested is a bit old. Normally, your code submission can avoid this bug.
>
> Then, we can also remove all the tests on the positiveness of tcfp_nkeys
> and the one you removed with your patch. WDYT?
Yes, remove tests on the positiveness of tcfp_nkeys in this case can also make code more robust,
In particular, at some abnormal situations. Should we do it now?
I will retest with your code merged, thanks.
>
> thanks,
> --
> davide
next prev parent reply other threads:[~2021-03-09 11:25 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-09 3:47 [PATCH] net/sched: act_pedit: fix a NULL pointer deref in tcf_pedit_init zhudi
2021-03-09 8:53 ` Davide Caratti
2021-03-09 11:24 ` zhudi (J) [this message]
2021-03-09 15:47 ` 答复: " Davide Caratti
2021-03-09 18:00 ` Cong Wang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=672f06766f2d49ecbb573037b3cb445a@huawei.com \
--to=zhudi21@huawei$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=dcaratti@redhat$(echo .)com \
--cc=jhs@mojatatu$(echo .)com \
--cc=kuba@kernel$(echo .)org \
--cc=netdev@vger$(echo .)kernel.org \
--cc=rose.chen@huawei$(echo .)com \
--cc=xiyou.wangcong@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox