linux-kernel - Re: [PATCH net] net/sched: Fix memory leak in tcindex_set

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 linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ad702330-d0da-c096-0f37-1cb4e866ddae@huawei.com>
Date:   Thu, 8 Dec 2022 14:35:37 +0800
From:   Chen Zhongjin <chenzhongjin@...wei.com>
To:     <syzbot+2f9183cb6f89b0e16586@...kaller.appspotmail.com>,
        <syzkaller-bugs@...glegroups.com>, <netdev@...r.kernel.org>,
        <stable@...r.kernel.org>, <linux-kernel@...r.kernel.org>
CC:     <jhs@...atatu.com>, <xiyou.wangcong@...il.com>, <jiri@...nulli.us>,
        <davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>,
        <pabeni@...hat.com>, <gregkh@...uxfoundation.org>
Subject: Re: [PATCH net] net/sched: Fix memory leak in tcindex_set_parms

Hi,

On 2022/12/8 11:22, Chen Zhongjin wrote:
> syzkaller reported a memleak:
> https://syzkaller.appspot.com/bug?id=e061e6cd46417ee6566dc249d8f982c0b5977a52
>
> unreferenced object 0xffff888107813900 (size 256):
>    backtrace:
>      kcalloc include/linux/slab.h:636 [inline]
>      tcf_exts_init include/net/pkt_cls.h:250 [inline]
>      tcindex_set_parms+0xa7/0xbe0 net/sched/cls_tcindex.c:342
>      tcindex_change+0xdf/0x120 net/sched/cls_tcindex.c:553
>      tc_new_tfilter+0x4f2/0x1100 net/sched/cls_api.c:2147
>      ...
>
> The reproduce calls tc_new_tfilter() continuously:
>
> tc_new_tfilter()...
> tcindex_set_parms()
>    tcf_exts_init(&e, ...) // alloc e->actions
>    tcf_exts_change(&r->exts, &e)
>
> tc_new_tfilter()...
> tcindex_set_parms()
>    old_r = r // same as first r
>    tcindex_filter_result_init(old_r, cp, net);
>    // old_r is holding e->actions but here it calls memset(old_r, 0)
>    // so the previous e->actions is leaked
>
> So here tcf_exts_destroy() should be called to free old_r->exts.actions
> before memset(old_r, 0) sets it to NULL.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@...r.kernel.org
> Reported-by: syzbot+2f9183cb6f89b0e16586@...kaller.appspotmail.com
> Signed-off-by: Chen Zhongjin <chenzhongjin@...wei.com>
> ---
> #syz test https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 355479c70a48
> ---
>   net/sched/cls_tcindex.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c
> index 1c9eeb98d826..00a6c04a4b42 100644
> --- a/net/sched/cls_tcindex.c
> +++ b/net/sched/cls_tcindex.c
> @@ -479,6 +479,7 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base,
>   	}
>   
>   	if (old_r && old_r != r) {
> +		tcf_exts_destroy(&old_r->exts);
>   		err = tcindex_filter_result_init(old_r, cp, net);
>   		if (err < 0) {
>   			kfree(f);

Just noticed that Hawkins has sent a patch for this. Please ignore mine.

Thanks!