Skip to content
This repository was archived by the owner on Jun 4, 2020. It is now read-only.

Commit 2f8e1d0

Browse files
congwanggregkh
authored andcommitted
igmp: acquire pmc lock for ip_mc_clear_src()
[ Upstream commit c38b7d327aafd1e3ad7ff53eefac990673b65667 ] Andrey reported a use-after-free in add_grec(): for (psf = *psf_list; psf; psf = psf_next) { ... psf_next = psf->sf_next; where the struct ip_sf_list's were already freed by: kfree+0xe8/0x2b0 mm/slub.c:3882 ip_mc_clear_src+0x69/0x1c0 net/ipv4/igmp.c:2078 ip_mc_dec_group+0x19a/0x470 net/ipv4/igmp.c:1618 ip_mc_drop_socket+0x145/0x230 net/ipv4/igmp.c:2609 inet_release+0x4e/0x1c0 net/ipv4/af_inet.c:411 sock_release+0x8d/0x1e0 net/socket.c:597 sock_close+0x16/0x20 net/socket.c:1072 This happens because we don't hold pmc->lock in ip_mc_clear_src() and a parallel mr_ifc_timer timer could jump in and access them. The RCU lock is there but it is merely for pmc itself, this spinlock could actually ensure we don't access them in parallel. Thanks to Eric and Long for discussion on this bug. Reported-by: Andrey Konovalov <andreyknvl@google.com> Cc: Eric Dumazet <edumazet@google.com> Cc: Xin Long <lucien.xin@gmail.com> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com> Reviewed-by: Xin Long <lucien.xin@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent e1f1dea commit 2f8e1d0

1 file changed

Lines changed: 13 additions & 8 deletions

File tree

net/ipv4/igmp.c

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1832,21 +1832,26 @@ static int ip_mc_add_src(struct in_device *in_dev, __be32 *pmca, int sfmode,
18321832

18331833
static void ip_mc_clear_src(struct ip_mc_list *pmc)
18341834
{
1835-
struct ip_sf_list *psf, *nextpsf;
1835+
struct ip_sf_list *psf, *nextpsf, *tomb, *sources;
18361836

1837-
for (psf = pmc->tomb; psf; psf = nextpsf) {
1837+
spin_lock_bh(&pmc->lock);
1838+
tomb = pmc->tomb;
1839+
pmc->tomb = NULL;
1840+
sources = pmc->sources;
1841+
pmc->sources = NULL;
1842+
pmc->sfmode = MCAST_EXCLUDE;
1843+
pmc->sfcount[MCAST_INCLUDE] = 0;
1844+
pmc->sfcount[MCAST_EXCLUDE] = 1;
1845+
spin_unlock_bh(&pmc->lock);
1846+
1847+
for (psf = tomb; psf; psf = nextpsf) {
18381848
nextpsf = psf->sf_next;
18391849
kfree(psf);
18401850
}
1841-
pmc->tomb = NULL;
1842-
for (psf = pmc->sources; psf; psf = nextpsf) {
1851+
for (psf = sources; psf; psf = nextpsf) {
18431852
nextpsf = psf->sf_next;
18441853
kfree(psf);
18451854
}
1846-
pmc->sources = NULL;
1847-
pmc->sfmode = MCAST_EXCLUDE;
1848-
pmc->sfcount[MCAST_INCLUDE] = 0;
1849-
pmc->sfcount[MCAST_EXCLUDE] = 1;
18501855
}
18511856

18521857

0 commit comments

Comments
 (0)