Received-SPF: pass (google.com: domain of linux-kernel+bounces-12174-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Precedence: bulk
MIME-Version: 1.0
References: <20231224165413.831486-1-linma@zju.edu.cn> <CAM0EoMm8F3UE3N-PBZmJHQpYYjiV23JKf6jGsvzzWs0PBd+AWQ@mail.gmail.com>
 <6aab36aa.56337.18ca3c6af7a.Coremail.linma@zju.edu.cn> <CAM0EoMmBp6SWDGhPkusnx0jh4y=1k9ggS+5UpV+0MtEccDgyXw@mail.gmail.com>
In-Reply-To: <CAM0EoMmBp6SWDGhPkusnx0jh4y=1k9ggS+5UpV+0MtEccDgyXw@mail.gmail.com>
From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: Wed, 27 Dec 2023 12:33:57 -0500
Message-ID: <CAM0EoMm5ruiBfJC1C+Jvz=vuKz03e_KOS_jgbS7ETkSfV-SKDA@mail.gmail.com>
Subject: Re: [PATCH net v1] net/sched: cls_api: complement tcf_tfilter_dump_policy
To: Lin Ma <linma@zju.edu.cn>
Cc: xiyou.wangcong@gmail.com, jiri@resnulli.us, davem@davemloft.net, 
	edumazet@google.com, kuba@kernel.org, pabeni@redhat.com, 
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Dec 27, 2023 at 12:02=E2=80=AFPM Jamal Hadi Salim <jhs@mojatatu.com=
> wrote:
>
> On Mon, Dec 25, 2023 at 8:39=E2=80=AFPM Lin Ma <linma@zju.edu.cn> wrote:
> >
> > Hello Jamal,
> >
> > >
> > > Can you clarify what "heap data leak" you are referring to?
> > > As much as i can see any reference to NLA_TCA_CHAIN is checked for
> > > presence before being put to use. So far that reason I  dont see how
> > > this patch qualifies as "net". It looks like an enhancement to me
> > > which should target net-next, unless i am missing something obvious.
> > >
> >
> > Sure, thanks for your reply, (and merry Christmas :D).
> > I didn't mention the detail as I consider the commit message in
> > `5e2424708da7` could make a point. In short, the code
> >
> > ```
> > if (tca[TCA_CHAIN] && nla_get_u32(tca[TCA_CHAIN])
> > ```
> >
> > only checks if the attribute TCA_CHAIN exists but never checks about
> > the attribute length because that attribute is parsed by the function
> > nlmsg_parse_deprecated which will parse an attribute even not described
> > in the given policy (here, the tcf_tfilter_dump_policy).
> >
> > Moreover, the netlink message is allocated via netlink_alloc_large_skb
> > (see net/netlink/af_netlink.c) that does not clear out the heap buffer.
> > Hence a malicious user could send a malicious TCA_CHAIN attribute here
> > without putting any payload and the above `nla_get_u32` could dereferen=
ce
> > a dirty data that is sprayed by the user.
> >
> > Other place gets TCA_CHAIN with provide policy rtm_tca_policy that has =
a
> > description.
> >
> > ```
> > [TCA_CHAIN]             =3D { .type =3D NLA_U32 },
> > ```
> >
> > and this patch aims to do so.
> >
> > Unfortunately, I have not opened the exploit for CVE-2023-3773
> > (https://access.redhat.com/security/cve/cve-2023-3773) yet but the idea
> > is similar and you can take it as an example.
> >
>
> Sorry, still trying to follow your reasoning that this is a "net issue":
> As you point out, the skb will have enough space to carry the 32 bit
> value. Worst case is we read garbage. And the dump, using this garbage
> chain index,  will not find the chain or will find some unintended
> chain. Am i missing something?
>
> Can you send me a repro (privately) that actually causes the "heap
> data leak" if you have one?
>

To clarify what triggered me is your tie of this as an exploit and
quoting CVEs. Maybe not so much net vs net-next.

cheers,
jamal

> cheers,
> jamal
>
>
> > > cheers,
> > > jamal
> > >
> >
> > Regards
> > Lin