diff mbox series

[linux-5.9,1/1] net: netfilter: fix KASAN: slab-out-of-bounds Read in nft_flow_rule_create

Message ID 20201019172532.3906-1-saeed.mirzamohammadi@oracle.com (mailing list archive)
State Not Applicable
Delegated to: Netdev Maintainers
Headers show
Series [linux-5.9,1/1] net: netfilter: fix KASAN: slab-out-of-bounds Read in nft_flow_rule_create | expand

Commit Message

Saeed Mirzamohammadi Oct. 19, 2020, 5:25 p.m. UTC 
From: Saeed Mirzamohammadi <saeed.mirzamohammadi@oracle.com>

This patch fixes the issue due to:

BUG: KASAN: slab-out-of-bounds in nft_flow_rule_create+0x622/0x6a2
net/netfilter/nf_tables_offload.c:40
Read of size 8 at addr ffff888103910b58 by task syz-executor227/16244

The error happens when expr->ops is accessed early on before performing the boundary check and after nft_expr_next() moves the expr to go out-of-bounds.

This patch checks the boundary condition before expr->ops that fixes the slab-out-of-bounds Read issue.

Signed-off-by: Saeed Mirzamohammadi <saeed.mirzamohammadi@oracle.com>
---
 net/netfilter/nf_tables_offload.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Pablo Neira Ayuso Oct. 20, 2020, 11:50 a.m. UTC | #1 
On Mon, Oct 19, 2020 at 10:25:32AM -0700, saeed.mirzamohammadi@oracle.com wrote:
> From: Saeed Mirzamohammadi <saeed.mirzamohammadi@oracle.com>
> 
> This patch fixes the issue due to:
> 
> BUG: KASAN: slab-out-of-bounds in nft_flow_rule_create+0x622/0x6a2
> net/netfilter/nf_tables_offload.c:40
> Read of size 8 at addr ffff888103910b58 by task syz-executor227/16244
> 
> The error happens when expr->ops is accessed early on before performing the boundary check and after nft_expr_next() moves the expr to go out-of-bounds.
> 
> This patch checks the boundary condition before expr->ops that fixes the slab-out-of-bounds Read issue.

Thanks. I made a slight variant of your patch.

I'm attaching it, it is also fixing the problem but it introduced
nft_expr_more() and use it everywhere.

Let me know if this looks fine to you.
Saeed Mirzamohammadi Oct. 20, 2020, 4:45 p.m. UTC | #2 
Thanks! Yes, that looks good to me.

Saeed

> On Oct 20, 2020, at 4:50 AM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> 
> On Mon, Oct 19, 2020 at 10:25:32AM -0700, saeed.mirzamohammadi@oracle.com wrote:
>> From: Saeed Mirzamohammadi <saeed.mirzamohammadi@oracle.com>
>> 
>> This patch fixes the issue due to:
>> 
>> BUG: KASAN: slab-out-of-bounds in nft_flow_rule_create+0x622/0x6a2
>> net/netfilter/nf_tables_offload.c:40
>> Read of size 8 at addr ffff888103910b58 by task syz-executor227/16244
>> 
>> The error happens when expr->ops is accessed early on before performing the boundary check and after nft_expr_next() moves the expr to go out-of-bounds.
>> 
>> This patch checks the boundary condition before expr->ops that fixes the slab-out-of-bounds Read issue.
> 
> Thanks. I made a slight variant of your patch.
> 
> I'm attaching it, it is also fixing the problem but it introduced
> nft_expr_more() and use it everywhere.
> 
> Let me know if this looks fine to you.
> <0001-netfilter-fix-KASAN-slab-out-of-bounds-Read-in-nft_f.patch>
Saeed Mirzamohammadi Oct. 21, 2020, 8:08 p.m. UTC | #3 
Attached the syzkaller C repro.

Tested-by: Saeed Mirzamohammadi <saeed.mirzamohammadi@oracle.com>
> On Oct 20, 2020, at 9:45 AM, Saeed Mirzamohammadi <saeed.mirzamohammadi@oracle.com> wrote:
> 
> Thanks! Yes, that looks good to me.
> 
> Saeed
> 
>> On Oct 20, 2020, at 4:50 AM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>> 
>> On Mon, Oct 19, 2020 at 10:25:32AM -0700, saeed.mirzamohammadi@oracle.com wrote:
>>> From: Saeed Mirzamohammadi <saeed.mirzamohammadi@oracle.com>
>>> 
>>> This patch fixes the issue due to:
>>> 
>>> BUG: KASAN: slab-out-of-bounds in nft_flow_rule_create+0x622/0x6a2
>>> net/netfilter/nf_tables_offload.c:40
>>> Read of size 8 at addr ffff888103910b58 by task syz-executor227/16244
>>> 
>>> The error happens when expr->ops is accessed early on before performing the boundary check and after nft_expr_next() moves the expr to go out-of-bounds.
>>> 
>>> This patch checks the boundary condition before expr->ops that fixes the slab-out-of-bounds Read issue.
>> 
>> Thanks. I made a slight variant of your patch.
>> 
>> I'm attaching it, it is also fixing the problem but it introduced
>> nft_expr_more() and use it everywhere.
>> 
>> Let me know if this looks fine to you.
>> <0001-netfilter-fix-KASAN-slab-out-of-bounds-Read-in-nft_f.patch>
>
Saeed Mirzamohammadi Oct. 25, 2020, 11:31 p.m. UTC | #4 
Adding stable.


> On Oct 21, 2020, at 1:08 PM, Saeed Mirzamohammadi <saeed.mirzamohammadi@oracle.com> wrote:
> 
> Attached the syzkaller C repro.
> 
> Tested-by: Saeed Mirzamohammadi <saeed.mirzamohammadi@oracle.com>
> <repro.c>
>> On Oct 20, 2020, at 9:45 AM, Saeed Mirzamohammadi <saeed.mirzamohammadi@oracle.com> wrote:
>> 
>> Thanks! Yes, that looks good to me.
>> 
>> Saeed
>> 
>>> On Oct 20, 2020, at 4:50 AM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>>> 
>>> On Mon, Oct 19, 2020 at 10:25:32AM -0700, saeed.mirzamohammadi@oracle.com wrote:
>>>> From: Saeed Mirzamohammadi <saeed.mirzamohammadi@oracle.com>
>>>> 
>>>> This patch fixes the issue due to:
>>>> 
>>>> BUG: KASAN: slab-out-of-bounds in nft_flow_rule_create+0x622/0x6a2
>>>> net/netfilter/nf_tables_offload.c:40
>>>> Read of size 8 at addr ffff888103910b58 by task syz-executor227/16244
>>>> 
>>>> The error happens when expr->ops is accessed early on before performing the boundary check and after nft_expr_next() moves the expr to go out-of-bounds.
>>>> 
>>>> This patch checks the boundary condition before expr->ops that fixes the slab-out-of-bounds Read issue.
>>> 
>>> Thanks. I made a slight variant of your patch.
>>> 
>>> I'm attaching it, it is also fixing the problem but it introduced
>>> nft_expr_more() and use it everywhere.
>>> 
>>> Let me know if this looks fine to you.
>>> <0001-netfilter-fix-KASAN-slab-out-of-bounds-Read-in-nft_f.patch>
>> 
>
Greg KH Oct. 27, 2020, 6:21 a.m. UTC | #5 
On Sun, Oct 25, 2020 at 04:31:57PM -0700, Saeed Mirzamohammadi wrote:
> Adding stable.

What did that do?

confused,

greg k-h
Pablo Neira Ayuso Oct. 27, 2020, 8:19 a.m. UTC | #6 
Hi Greg,

On Tue, Oct 27, 2020 at 07:21:11AM +0100, Greg KH wrote:
> On Sun, Oct 25, 2020 at 04:31:57PM -0700, Saeed Mirzamohammadi wrote:
> > Adding stable.
> 
> What did that do?

Saeed is requesting that stable maintainers cherry-picks this patch:

31cc578ae2de ("netfilter: nftables_offload: KASAN slab-out-of-bounds
Read in nft_flow_rule_create")

into stable 5.4 and 5.8.

Thanks.
Greg KH Oct. 29, 2020, 11:02 a.m. UTC | #7 
On Tue, Oct 27, 2020 at 09:19:22AM +0100, Pablo Neira Ayuso wrote:
> Hi Greg,
> 
> On Tue, Oct 27, 2020 at 07:21:11AM +0100, Greg KH wrote:
> > On Sun, Oct 25, 2020 at 04:31:57PM -0700, Saeed Mirzamohammadi wrote:
> > > Adding stable.
> > 
> > What did that do?
> 
> Saeed is requesting that stable maintainers cherry-picks this patch:
> 
> 31cc578ae2de ("netfilter: nftables_offload: KASAN slab-out-of-bounds
> Read in nft_flow_rule_create")
> 
> into stable 5.4 and 5.8.

5.9 is also a stable kernel :)

Will go queue it up everywhere...

thanks,

greg k-h
Pablo Neira Ayuso Oct. 29, 2020, 11:06 a.m. UTC | #8 
On Thu, Oct 29, 2020 at 12:02:41PM +0100, Greg KH wrote:
> On Tue, Oct 27, 2020 at 09:19:22AM +0100, Pablo Neira Ayuso wrote:
> > Hi Greg,
> > 
> > On Tue, Oct 27, 2020 at 07:21:11AM +0100, Greg KH wrote:
> > > On Sun, Oct 25, 2020 at 04:31:57PM -0700, Saeed Mirzamohammadi wrote:
> > > > Adding stable.
> > > 
> > > What did that do?
> > 
> > Saeed is requesting that stable maintainers cherry-picks this patch:
> > 
> > 31cc578ae2de ("netfilter: nftables_offload: KASAN slab-out-of-bounds
> > Read in nft_flow_rule_create")
> > 
> > into stable 5.4 and 5.8.
> 
> 5.9 is also a stable kernel :)

Oh, indeed, I forgot this one :)

> Will go queue it up everywhere...

Thanks.
diff mbox series

Patch

diff --git a/net/netfilter/nf_tables_offload.c b/net/netfilter/nf_tables_offload.c
index 9ef37c1b7b3b..1273e3c0d4b8 100644
--- a/net/netfilter/nf_tables_offload.c
+++ b/net/netfilter/nf_tables_offload.c
@@ -37,7 +37,7 @@  struct nft_flow_rule *nft_flow_rule_create(struct net *net,
 	struct nft_expr *expr;
 
 	expr = nft_expr_first(rule);
-	while (expr->ops && expr != nft_expr_last(rule)) {
+	while (expr != nft_expr_last(rule) && expr->ops) {
 		if (expr->ops->offload_flags & NFT_OFFLOAD_F_ACTION)
 			num_actions++;
 
@@ -61,7 +61,7 @@  struct nft_flow_rule *nft_flow_rule_create(struct net *net,
 	ctx->net = net;
 	ctx->dep.type = NFT_OFFLOAD_DEP_UNSPEC;
 
-	while (expr->ops && expr != nft_expr_last(rule)) {
+	while (expr != nft_expr_last(rule) && expr->ops) {
 		if (!expr->ops->offload) {
 			err = -EOPNOTSUPP;
 			goto err_out;