Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1733568 > unrolled thread

[PATCH for 4.9 01/39] net: core: Prevent from dereferencing null pointer when releasing SKB

Started by"Levin, Alexander (Sasha Levin)" <alexander.levin@verizon.com>
First post2017-09-18 02:30 +0200
Last post2017-09-18 16:40 +0200
Articles 7 — 2 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH for 4.9 01/39] net: core: Prevent from dereferencing null  pointer when releasing SKB "Levin, Alexander (Sasha Levin)" <alexander.levin@verizon.com> - 2017-09-18 02:30 +0200
    [PATCH for 4.9 10/39] Btrfs: fix segmentation fault when doing dio  read "Levin, Alexander (Sasha Levin)" <alexander.levin@verizon.com> - 2017-09-18 02:30 +0200
    [PATCH for 4.9 14/39] kasan: do not sanitize kexec purgatory "Levin, Alexander (Sasha Levin)" <alexander.levin@verizon.com> - 2017-09-18 02:30 +0200
    [PATCH for 4.9 16/39] netfilter: invoke synchronize_rcu after set the  _hook_ to NULL "Levin, Alexander (Sasha Levin)" <alexander.levin@verizon.com> - 2017-09-18 02:30 +0200
    Re: [PATCH for 4.9 01/39] net: core: Prevent from dereferencing null  pointer when releasing SKB Greg KH <greg@kroah.com> - 2017-09-18 08:50 +0200
      Re: [PATCH for 4.9 01/39] net: core: Prevent from dereferencing null  pointer when releasing SKB "Levin, Alexander (Sasha Levin)" <alexander.levin@verizon.com> - 2017-09-18 16:20 +0200
        Re: [PATCH for 4.9 01/39] net: core: Prevent from dereferencing null  pointer when releasing SKB "Levin, Alexander (Sasha Levin)" <alexander.levin@verizon.com> - 2017-09-18 16:40 +0200

#1733568 — [PATCH for 4.9 01/39] net: core: Prevent from dereferencing null pointer when releasing SKB

From"Levin, Alexander (Sasha Levin)" <alexander.levin@verizon.com>
Date2017-09-18 02:30 +0200
Subject[PATCH for 4.9 01/39] net: core: Prevent from dereferencing null pointer when releasing SKB
Message-ID<uqS6J-6Te-3@gated-at.bofh.it>
From: Myungho Jung <mhjungk@gmail.com>

[ Upstream commit 9899886d5e8ec5b343b1efe44f185a0e68dc6454 ]

Added NULL check to make __dev_kfree_skb_irq consistent with kfree
family of functions.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=195289

Signed-off-by: Myungho Jung <mhjungk@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <alexander.levin@verizon.com>
---
 net/core/dev.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/core/dev.c b/net/core/dev.c
index 1d0a7369d5a2..8d5db08f79f3 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2355,6 +2355,9 @@ void __dev_kfree_skb_irq(struct sk_buff *skb, enum skb_free_reason reason)
 {
 	unsigned long flags;
 
+	if (unlikely(!skb))
+		return;
+
 	if (likely(atomic_read(&skb->users) == 1)) {
 		smp_rmb();
 		atomic_set(&skb->users, 0);
-- 
2.11.0

[toc] | [next] | [standalone]

#1733569 — [PATCH for 4.9 10/39] Btrfs: fix segmentation fault when doing dio read

From"Levin, Alexander (Sasha Levin)" <alexander.levin@verizon.com>
Date2017-09-18 02:30 +0200
Subject[PATCH for 4.9 10/39] Btrfs: fix segmentation fault when doing dio read
Message-ID<uqS6L-6Te-37@gated-at.bofh.it>
In reply to#1733568
From: Liu Bo <bo.li.liu@oracle.com>

[ Upstream commit 97bf5a5589aa3a59c60aa775fc12ec0483fc5002 ]

Commit 2dabb3248453 ("Btrfs: Direct I/O read: Work on sectorsized blocks")
introduced this bug during iterating bio pages in dio read's endio hook,
and it could end up with segment fault of the dio reading task.

So the reason is 'if (nr_sectors--)', and it makes the code assume that
there is one more block in the same page, so page offset is increased and
the bio which is created to repair the bad block then has an incorrect
bvec.bv_offset, and a later access of the page content would throw a
segmentation fault.

This also adds ASSERT to check page offset against page size.

Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Signed-off-by: Sasha Levin <alexander.levin@verizon.com>
---
 fs/btrfs/inode.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 8a05fa7e2152..f089d7d8afe7 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -8050,8 +8050,10 @@ static int __btrfs_correct_data_nocsum(struct inode *inode,
 
 		start += sectorsize;
 
-		if (nr_sectors--) {
+		nr_sectors--;
+		if (nr_sectors) {
 			pgoff += sectorsize;
+			ASSERT(pgoff < PAGE_SIZE);
 			goto next_block_or_try_again;
 		}
 	}
@@ -8157,8 +8159,10 @@ static int __btrfs_subio_endio_read(struct inode *inode,
 
 		ASSERT(nr_sectors);
 
-		if (--nr_sectors) {
+		nr_sectors--;
+		if (nr_sectors) {
 			pgoff += sectorsize;
+			ASSERT(pgoff < PAGE_SIZE);
 			goto next_block;
 		}
 	}
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1733570 — [PATCH for 4.9 14/39] kasan: do not sanitize kexec purgatory

From"Levin, Alexander (Sasha Levin)" <alexander.levin@verizon.com>
Date2017-09-18 02:30 +0200
Subject[PATCH for 4.9 14/39] kasan: do not sanitize kexec purgatory
Message-ID<uqS6L-6Te-39@gated-at.bofh.it>
In reply to#1733568
From: Mike Galbraith <efault@gmx.de>

[ Upstream commit 13a6798e4a03096b11bf402a063786a7be55d426 ]

Fixes this:

  kexec: Undefined symbol: __asan_load8_noabort
  kexec-bzImage64: Loading purgatory failed

Link: http://lkml.kernel.org/r/1489672155.4458.7.camel@gmx.de
Signed-off-by: Mike Galbraith <efault@gmx.de>
Cc: Alexander Potapenko <glider@google.com>
Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Sasha Levin <alexander.levin@verizon.com>
---
 arch/x86/purgatory/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/purgatory/Makefile b/arch/x86/purgatory/Makefile
index 555b9fa0ad43..7dbdb780264d 100644
--- a/arch/x86/purgatory/Makefile
+++ b/arch/x86/purgatory/Makefile
@@ -8,6 +8,7 @@ PURGATORY_OBJS = $(addprefix $(obj)/,$(purgatory-y))
 LDFLAGS_purgatory.ro := -e purgatory_start -r --no-undefined -nostdlib -z nodefaultlib
 targets += purgatory.ro
 
+KASAN_SANITIZE	:= n
 KCOV_INSTRUMENT := n
 
 # Default KBUILD_CFLAGS can have -pg option set when FTRACE is enabled. That
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1733571 — [PATCH for 4.9 16/39] netfilter: invoke synchronize_rcu after set the _hook_ to NULL

From"Levin, Alexander (Sasha Levin)" <alexander.levin@verizon.com>
Date2017-09-18 02:30 +0200
Subject[PATCH for 4.9 16/39] netfilter: invoke synchronize_rcu after set the _hook_ to NULL
Message-ID<uqS6L-6Te-41@gated-at.bofh.it>
In reply to#1733568
From: Liping Zhang <zlpnobody@gmail.com>

[ Upstream commit 3b7dabf029478bb80507a6c4500ca94132a2bc0b ]

Otherwise, another CPU may access the invalid pointer. For example:
    CPU0                CPU1
     -              rcu_read_lock();
     -              pfunc = _hook_;
  _hook_ = NULL;          -
  mod unload              -
     -                 pfunc(); // invalid, panic
     -             rcu_read_unlock();

So we must call synchronize_rcu() to wait the rcu reader to finish.

Also note, in nf_nat_snmp_basic_fini, synchronize_rcu() will be invoked
by later nf_conntrack_helper_unregister, but I'm inclined to add a
explicit synchronize_rcu after set the nf_nat_snmp_hook to NULL. Depend
on such obscure assumptions is not a good idea.

Last, in nfnetlink_cttimeout, we use kfree_rcu to free the time object,
so in cttimeout_exit, invoking rcu_barrier() is not necessary at all,
remove it too.

Signed-off-by: Liping Zhang <zlpnobody@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Sasha Levin <alexander.levin@verizon.com>
---
 net/ipv4/netfilter/nf_nat_snmp_basic.c | 1 +
 net/netfilter/nf_conntrack_ecache.c    | 2 ++
 net/netfilter/nf_conntrack_netlink.c   | 1 +
 net/netfilter/nf_nat_core.c            | 2 ++
 net/netfilter/nfnetlink_cttimeout.c    | 2 +-
 5 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/netfilter/nf_nat_snmp_basic.c b/net/ipv4/netfilter/nf_nat_snmp_basic.c
index c9b52c361da2..5a8f7c360887 100644
--- a/net/ipv4/netfilter/nf_nat_snmp_basic.c
+++ b/net/ipv4/netfilter/nf_nat_snmp_basic.c
@@ -1304,6 +1304,7 @@ static int __init nf_nat_snmp_basic_init(void)
 static void __exit nf_nat_snmp_basic_fini(void)
 {
 	RCU_INIT_POINTER(nf_nat_snmp_hook, NULL);
+	synchronize_rcu();
 	nf_conntrack_helper_unregister(&snmp_trap_helper);
 }
 
diff --git a/net/netfilter/nf_conntrack_ecache.c b/net/netfilter/nf_conntrack_ecache.c
index da9df2d56e66..22fc32143e9c 100644
--- a/net/netfilter/nf_conntrack_ecache.c
+++ b/net/netfilter/nf_conntrack_ecache.c
@@ -290,6 +290,7 @@ void nf_conntrack_unregister_notifier(struct net *net,
 	BUG_ON(notify != new);
 	RCU_INIT_POINTER(net->ct.nf_conntrack_event_cb, NULL);
 	mutex_unlock(&nf_ct_ecache_mutex);
+	/* synchronize_rcu() is called from ctnetlink_exit. */
 }
 EXPORT_SYMBOL_GPL(nf_conntrack_unregister_notifier);
 
@@ -326,6 +327,7 @@ void nf_ct_expect_unregister_notifier(struct net *net,
 	BUG_ON(notify != new);
 	RCU_INIT_POINTER(net->ct.nf_expect_event_cb, NULL);
 	mutex_unlock(&nf_ct_ecache_mutex);
+	/* synchronize_rcu() is called from ctnetlink_exit. */
 }
 EXPORT_SYMBOL_GPL(nf_ct_expect_unregister_notifier);
 
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 04111c1c3988..d5caed5bcfb1 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -3413,6 +3413,7 @@ static void __exit ctnetlink_exit(void)
 #ifdef CONFIG_NETFILTER_NETLINK_GLUE_CT
 	RCU_INIT_POINTER(nfnl_ct_hook, NULL);
 #endif
+	synchronize_rcu();
 }
 
 module_init(ctnetlink_init);
diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c
index dde64c4565d2..2916f4815c9c 100644
--- a/net/netfilter/nf_nat_core.c
+++ b/net/netfilter/nf_nat_core.c
@@ -892,6 +892,8 @@ static void __exit nf_nat_cleanup(void)
 #ifdef CONFIG_XFRM
 	RCU_INIT_POINTER(nf_nat_decode_session_hook, NULL);
 #endif
+	synchronize_rcu();
+
 	for (i = 0; i < NFPROTO_NUMPROTO; i++)
 		kfree(nf_nat_l4protos[i]);
 
diff --git a/net/netfilter/nfnetlink_cttimeout.c b/net/netfilter/nfnetlink_cttimeout.c
index 139e0867e56e..47d6656c9119 100644
--- a/net/netfilter/nfnetlink_cttimeout.c
+++ b/net/netfilter/nfnetlink_cttimeout.c
@@ -646,8 +646,8 @@ static void __exit cttimeout_exit(void)
 #ifdef CONFIG_NF_CONNTRACK_TIMEOUT
 	RCU_INIT_POINTER(nf_ct_timeout_find_get_hook, NULL);
 	RCU_INIT_POINTER(nf_ct_timeout_put_hook, NULL);
+	synchronize_rcu();
 #endif /* CONFIG_NF_CONNTRACK_TIMEOUT */
-	rcu_barrier();
 }
 
 module_init(cttimeout_init);
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1733683

FromGreg KH <greg@kroah.com>
Date2017-09-18 08:50 +0200
Message-ID<uqY2u-2oA-29@gated-at.bofh.it>
In reply to#1733568
On Mon, Sep 18, 2017 at 12:19:36AM +0000, Levin, Alexander (Sasha Levin) wrote:
> From: Myungho Jung <mhjungk@gmail.com>
> 
> [ Upstream commit 9899886d5e8ec5b343b1efe44f185a0e68dc6454 ]
> 
> Added NULL check to make __dev_kfree_skb_irq consistent with kfree
> family of functions.
> 
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=195289
> 
> Signed-off-by: Myungho Jung <mhjungk@gmail.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> Signed-off-by: Sasha Levin <alexander.levin@verizon.com>

Is this a different series from your original XX/59 patch series that
you feel is ready to go into the stable tree, or are you still asking
for review for these before they get submitted?

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]

#1734171

From"Levin, Alexander (Sasha Levin)" <alexander.levin@verizon.com>
Date2017-09-18 16:20 +0200
Message-ID<ur53Y-7mI-1@gated-at.bofh.it>
In reply to#1733683
On Mon, Sep 18, 2017 at 08:41:02AM +0200, Greg KH wrote:
>On Mon, Sep 18, 2017 at 12:19:36AM +0000, Levin, Alexander (Sasha Levin) wrote:
>> From: Myungho Jung <mhjungk@gmail.com>
>>
>> [ Upstream commit 9899886d5e8ec5b343b1efe44f185a0e68dc6454 ]
>>
>> Added NULL check to make __dev_kfree_skb_irq consistent with kfree
>> family of functions.
>>
>> Link: https://urldefense.proofpoint.com/v2/url?u=https-3A__bugzilla.kernel.org_show-5Fbug.cgi-3Fid-3D195289&d=DwIBAg&c=udBTRvFvXC5Dhqg7UHpJlPps3mZ3LRxpb6__0PomBTQ&r=bUtaaC9mlBij4OjEG_D-KPul_335azYzfC4Rjgomobo&m=iXzciSQOaZF7sggj-m_1eQCbmqf43dsIy8ogFRdIvSE&s=kZBpt2uE3l0TtR50y0QmqJyb1Wp3A-FB-GVfkwnTgVI&e=
>>
>> Signed-off-by: Myungho Jung <mhjungk@gmail.com>
>> Signed-off-by: David S. Miller <davem@davemloft.net>
>> Signed-off-by: Sasha Levin <alexander.levin@verizon.com>
>
>Is this a different series from your original XX/59 patch series that
>you feel is ready to go into the stable tree, or are you still asking
>for review for these before they get submitted?

This is a different series. Figured I'd do 2-3 in parallel to speed up things.

-- 

Thanks,
Sasha

[toc] | [prev] | [next] | [standalone]

#1734187

From"Levin, Alexander (Sasha Levin)" <alexander.levin@verizon.com>
Date2017-09-18 16:40 +0200
Message-ID<ur5nk-7uE-7@gated-at.bofh.it>
In reply to#1734171
On Mon, Sep 18, 2017 at 10:13:21AM -0400, Sasha Levin wrote:
>On Mon, Sep 18, 2017 at 08:41:02AM +0200, Greg KH wrote:
>>On Mon, Sep 18, 2017 at 12:19:36AM +0000, Levin, Alexander (Sasha Levin) wrote:
>>>From: Myungho Jung <mhjungk@gmail.com>
>>>
>>>[ Upstream commit 9899886d5e8ec5b343b1efe44f185a0e68dc6454 ]
>>>
>>>Added NULL check to make __dev_kfree_skb_irq consistent with kfree
>>>family of functions.
>>>
>>>Link: https://urldefense.proofpoint.com/v2/url?u=https-3A__bugzilla.kernel.org_show-5Fbug.cgi-3Fid-3D195289&d=DwIBAg&c=udBTRvFvXC5Dhqg7UHpJlPps3mZ3LRxpb6__0PomBTQ&r=bUtaaC9mlBij4OjEG_D-KPul_335azYzfC4Rjgomobo&m=iXzciSQOaZF7sggj-m_1eQCbmqf43dsIy8ogFRdIvSE&s=kZBpt2uE3l0TtR50y0QmqJyb1Wp3A-FB-GVfkwnTgVI&e=
>>>
>>>Signed-off-by: Myungho Jung <mhjungk@gmail.com>
>>>Signed-off-by: David S. Miller <davem@davemloft.net>
>>>Signed-off-by: Sasha Levin <alexander.levin@verizon.com>
>>
>>Is this a different series from your original XX/59 patch series that
>>you feel is ready to go into the stable tree, or are you still asking
>>for review for these before they get submitted?
>
>This is a different series. Figured I'd do 2-3 in parallel to speed up things.

Sorry, just to clarify, I'm waiting for reviews on this one.

I'll improve the patch subject prefix next time to clearify that.

-- 

Thanks,
Sasha

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web