* [PATCH net] packet: avoid panic in packet_getsockopt()
@ 2017-10-18 22:08 Eric Dumazet
2017-10-18 23:14 ` [PATCH v2 " Eric Dumazet
2017-10-21 0:50 ` [PATCH " David Miller
0 siblings, 2 replies; 7+ messages in thread
From: Eric Dumazet @ 2017-10-18 22:08 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Willem de Bruijn, Eric Dumazet
From: Eric Dumazet <edumazet@google.com>
syzkaller got crashes in packet_getsockopt() processing
PACKET_ROLLOVER_STATS command while another thread was managing
to change po->rollover
Using RCU will fix this bug. We might later add proper RCU annotations
for sparse sake.
Fixes: a9b6391814d5 ("packet: rollover statistics")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Willem de Bruijn <willemb@google.com>
---
net/packet/af_packet.c | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index bec01a3daf5b02bd716dbff5c9efef8d6a7982be..1d8a7add86b4f29880e11c6f4971d79319dcb426 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1796,8 +1796,10 @@ static struct packet_fanout *fanout_release(struct sock *sk)
else
f = NULL;
- if (po->rollover)
+ if (po->rollover) {
kfree_rcu(po->rollover, rcu);
+ po->rollover = NULL;
+ }
}
mutex_unlock(&fanout_mutex);
@@ -3851,6 +3853,7 @@ static int packet_getsockopt(struct socket *sock, int level, int optname,
void *data = &val;
union tpacket_stats_u st;
struct tpacket_rollover_stats rstats;
+ struct packet_rollover *rollover;
if (level != SOL_PACKET)
return -ENOPROTOOPT;
@@ -3929,13 +3932,18 @@ static int packet_getsockopt(struct socket *sock, int level, int optname,
0);
break;
case PACKET_ROLLOVER_STATS:
- if (!po->rollover)
+ rcu_read_lock();
+ rollover = rcu_dereference(po->rollover);
+ if (rollover) {
+ rstats.tp_all = atomic_long_read(&rollover->num);
+ rstats.tp_huge = atomic_long_read(&rollover->num_huge);
+ rstats.tp_failed = atomic_long_read(&rollover->num_failed);
+ data = &rstats;
+ lv = sizeof(rstats);
+ }
+ rcu_read_unlock();
+ if (!rollover)
return -EINVAL;
- rstats.tp_all = atomic_long_read(&po->rollover->num);
- rstats.tp_huge = atomic_long_read(&po->rollover->num_huge);
- rstats.tp_failed = atomic_long_read(&po->rollover->num_failed);
- data = &rstats;
- lv = sizeof(rstats);
break;
case PACKET_TX_HAS_OFF:
val = po->tp_tx_has_off;
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v2 net] packet: avoid panic in packet_getsockopt()
2017-10-18 22:08 [PATCH net] packet: avoid panic in packet_getsockopt() Eric Dumazet
@ 2017-10-18 23:14 ` Eric Dumazet
2017-10-20 19:11 ` kbuild test robot
2017-10-21 0:50 ` [PATCH " David Miller
1 sibling, 1 reply; 7+ messages in thread
From: Eric Dumazet @ 2017-10-18 23:14 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Willem de Bruijn, Eric Dumazet, John Sperbeck
From: Eric Dumazet <edumazet@google.com>
syzkaller got crashes in packet_getsockopt() processing
PACKET_ROLLOVER_STATS command while another thread was managing
to change po->rollover
Using RCU will fix this bug. We might later add proper RCU annotations
for sparse sake.
In v2: I replaced kfree(rollover) in fanout_add() to kfree_rcu()
variant, as spotted by John.
Fixes: a9b6391814d5 ("packet: rollover statistics")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Willem de Bruijn <willemb@google.com>
Cc: John Sperbeck <jsperbeck@google.com>
---
net/packet/af_packet.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index bec01a3daf5b02bd716dbff5c9efef8d6a7982be..2986941164b1952b3b6014ff81d2986b504c334a 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1769,7 +1769,7 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
out:
if (err && rollover) {
- kfree(rollover);
+ kfree_rcu(rollover, rcu);
po->rollover = NULL;
}
mutex_unlock(&fanout_mutex);
@@ -1796,8 +1796,10 @@ static struct packet_fanout *fanout_release(struct sock *sk)
else
f = NULL;
- if (po->rollover)
+ if (po->rollover) {
kfree_rcu(po->rollover, rcu);
+ po->rollover = NULL;
+ }
}
mutex_unlock(&fanout_mutex);
@@ -3851,6 +3853,7 @@ static int packet_getsockopt(struct socket *sock, int level, int optname,
void *data = &val;
union tpacket_stats_u st;
struct tpacket_rollover_stats rstats;
+ struct packet_rollover *rollover;
if (level != SOL_PACKET)
return -ENOPROTOOPT;
@@ -3929,13 +3932,18 @@ static int packet_getsockopt(struct socket *sock, int level, int optname,
0);
break;
case PACKET_ROLLOVER_STATS:
- if (!po->rollover)
+ rcu_read_lock();
+ rollover = rcu_dereference(po->rollover);
+ if (rollover) {
+ rstats.tp_all = atomic_long_read(&rollover->num);
+ rstats.tp_huge = atomic_long_read(&rollover->num_huge);
+ rstats.tp_failed = atomic_long_read(&rollover->num_failed);
+ data = &rstats;
+ lv = sizeof(rstats);
+ }
+ rcu_read_unlock();
+ if (!rollover)
return -EINVAL;
- rstats.tp_all = atomic_long_read(&po->rollover->num);
- rstats.tp_huge = atomic_long_read(&po->rollover->num_huge);
- rstats.tp_failed = atomic_long_read(&po->rollover->num_failed);
- data = &rstats;
- lv = sizeof(rstats);
break;
case PACKET_TX_HAS_OFF:
val = po->tp_tx_has_off;
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2 net] packet: avoid panic in packet_getsockopt()
2017-10-18 23:14 ` [PATCH v2 " Eric Dumazet
@ 2017-10-20 19:11 ` kbuild test robot
2017-10-20 19:25 ` Eric Dumazet
0 siblings, 1 reply; 7+ messages in thread
From: kbuild test robot @ 2017-10-20 19:11 UTC (permalink / raw)
To: Eric Dumazet
Cc: kbuild-all, David Miller, netdev, Willem de Bruijn, Eric Dumazet,
John Sperbeck
Hi Eric,
[auto build test WARNING on net/master]
url: https://github.com/0day-ci/linux/commits/Eric-Dumazet/packet-avoid-panic-in-packet_getsockopt/20171021-003615
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
vim +3936 net/packet/af_packet.c
3845
3846 static int packet_getsockopt(struct socket *sock, int level, int optname,
3847 char __user *optval, int __user *optlen)
3848 {
3849 int len;
3850 int val, lv = sizeof(val);
3851 struct sock *sk = sock->sk;
3852 struct packet_sock *po = pkt_sk(sk);
3853 void *data = &val;
3854 union tpacket_stats_u st;
3855 struct tpacket_rollover_stats rstats;
3856 struct packet_rollover *rollover;
3857
3858 if (level != SOL_PACKET)
3859 return -ENOPROTOOPT;
3860
3861 if (get_user(len, optlen))
3862 return -EFAULT;
3863
3864 if (len < 0)
3865 return -EINVAL;
3866
3867 switch (optname) {
3868 case PACKET_STATISTICS:
3869 spin_lock_bh(&sk->sk_receive_queue.lock);
3870 memcpy(&st, &po->stats, sizeof(st));
3871 memset(&po->stats, 0, sizeof(po->stats));
3872 spin_unlock_bh(&sk->sk_receive_queue.lock);
3873
3874 if (po->tp_version == TPACKET_V3) {
3875 lv = sizeof(struct tpacket_stats_v3);
3876 st.stats3.tp_packets += st.stats3.tp_drops;
3877 data = &st.stats3;
3878 } else {
3879 lv = sizeof(struct tpacket_stats);
3880 st.stats1.tp_packets += st.stats1.tp_drops;
3881 data = &st.stats1;
3882 }
3883
3884 break;
3885 case PACKET_AUXDATA:
3886 val = po->auxdata;
3887 break;
3888 case PACKET_ORIGDEV:
3889 val = po->origdev;
3890 break;
3891 case PACKET_VNET_HDR:
3892 val = po->has_vnet_hdr;
3893 break;
3894 case PACKET_VERSION:
3895 val = po->tp_version;
3896 break;
3897 case PACKET_HDRLEN:
3898 if (len > sizeof(int))
3899 len = sizeof(int);
3900 if (len < sizeof(int))
3901 return -EINVAL;
3902 if (copy_from_user(&val, optval, len))
3903 return -EFAULT;
3904 switch (val) {
3905 case TPACKET_V1:
3906 val = sizeof(struct tpacket_hdr);
3907 break;
3908 case TPACKET_V2:
3909 val = sizeof(struct tpacket2_hdr);
3910 break;
3911 case TPACKET_V3:
3912 val = sizeof(struct tpacket3_hdr);
3913 break;
3914 default:
3915 return -EINVAL;
3916 }
3917 break;
3918 case PACKET_RESERVE:
3919 val = po->tp_reserve;
3920 break;
3921 case PACKET_LOSS:
3922 val = po->tp_loss;
3923 break;
3924 case PACKET_TIMESTAMP:
3925 val = po->tp_tstamp;
3926 break;
3927 case PACKET_FANOUT:
3928 val = (po->fanout ?
3929 ((u32)po->fanout->id |
3930 ((u32)po->fanout->type << 16) |
3931 ((u32)po->fanout->flags << 24)) :
3932 0);
3933 break;
3934 case PACKET_ROLLOVER_STATS:
3935 rcu_read_lock();
> 3936 rollover = rcu_dereference(po->rollover);
3937 if (rollover) {
3938 rstats.tp_all = atomic_long_read(&rollover->num);
3939 rstats.tp_huge = atomic_long_read(&rollover->num_huge);
3940 rstats.tp_failed = atomic_long_read(&rollover->num_failed);
3941 data = &rstats;
3942 lv = sizeof(rstats);
3943 }
3944 rcu_read_unlock();
3945 if (!rollover)
3946 return -EINVAL;
3947 break;
3948 case PACKET_TX_HAS_OFF:
3949 val = po->tp_tx_has_off;
3950 break;
3951 case PACKET_QDISC_BYPASS:
3952 val = packet_use_direct_xmit(po);
3953 break;
3954 default:
3955 return -ENOPROTOOPT;
3956 }
3957
3958 if (len > lv)
3959 len = lv;
3960 if (put_user(len, optlen))
3961 return -EFAULT;
3962 if (copy_to_user(optval, data, len))
3963 return -EFAULT;
3964 return 0;
3965 }
3966
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2 net] packet: avoid panic in packet_getsockopt()
2017-10-20 19:11 ` kbuild test robot
@ 2017-10-20 19:25 ` Eric Dumazet
0 siblings, 0 replies; 7+ messages in thread
From: Eric Dumazet @ 2017-10-20 19:25 UTC (permalink / raw)
To: kbuild test robot
Cc: Eric Dumazet, kbuild-all, David Miller, netdev, Willem de Bruijn,
John Sperbeck
On Fri, Oct 20, 2017 at 12:11 PM, kbuild test robot <lkp@intel.com> wrote:
> Hi Eric,
>
> [auto build test WARNING on net/master]
>
> url: https://github.com/0day-ci/linux/commits/Eric-Dumazet/packet-avoid-panic-in-packet_getsockopt/20171021-003615
> reproduce:
> # apt-get install sparse
> make ARCH=x86_64 allmodconfig
> make C=1 CF=-D__CHECK_ENDIAN__
>
>
> sparse warnings: (new ones prefixed by >>)
>
Thanks, this was mentioned in the commit changelog.
We will add rcu annotations in net-next.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] packet: avoid panic in packet_getsockopt()
2017-10-18 22:08 [PATCH net] packet: avoid panic in packet_getsockopt() Eric Dumazet
2017-10-18 23:14 ` [PATCH v2 " Eric Dumazet
@ 2017-10-21 0:50 ` David Miller
2017-10-21 0:53 ` David Miller
1 sibling, 1 reply; 7+ messages in thread
From: David Miller @ 2017-10-21 0:50 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, willemb, edumazet
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 18 Oct 2017 15:08:24 -0700
> From: Eric Dumazet <edumazet@google.com>
>
> syzkaller got crashes in packet_getsockopt() processing
> PACKET_ROLLOVER_STATS command while another thread was managing
> to change po->rollover
>
> Using RCU will fix this bug. We might later add proper RCU annotations
> for sparse sake.
>
> Fixes: a9b6391814d5 ("packet: rollover statistics")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied and queued up for -stable, but:
> Willem de Bruijn <willemb@google.com>
I didn't know what kind of tag you intended to use here for Willem
so just deleted the line.
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH net] packet: avoid panic in packet_getsockopt()
2017-10-21 0:50 ` [PATCH " David Miller
@ 2017-10-21 0:53 ` David Miller
2017-10-21 1:11 ` Eric Dumazet
0 siblings, 1 reply; 7+ messages in thread
From: David Miller @ 2017-10-21 0:53 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, willemb, edumazet
From: David Miller <davem@davemloft.net>
Date: Sat, 21 Oct 2017 01:50:08 +0100 (WEST)
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Wed, 18 Oct 2017 15:08:24 -0700
>
>> From: Eric Dumazet <edumazet@google.com>
>>
>> syzkaller got crashes in packet_getsockopt() processing
>> PACKET_ROLLOVER_STATS command while another thread was managing
>> to change po->rollover
>>
>> Using RCU will fix this bug. We might later add proper RCU annotations
>> for sparse sake.
>>
>> Fixes: a9b6391814d5 ("packet: rollover statistics")
>> Signed-off-by: Eric Dumazet <edumazet@google.com>
>
> Applied and queued up for -stable, but:
>
>> Willem de Bruijn <willemb@google.com>
>
> I didn't know what kind of tag you intended to use here for Willem
> so just deleted the line.
I just noticed 'v2' of this patch and replaced it in time before I
pushed out :-)
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH net] packet: avoid panic in packet_getsockopt()
2017-10-21 0:53 ` David Miller
@ 2017-10-21 1:11 ` Eric Dumazet
0 siblings, 0 replies; 7+ messages in thread
From: Eric Dumazet @ 2017-10-21 1:11 UTC (permalink / raw)
To: David Miller; +Cc: Eric Dumazet, netdev, Willem de Bruijn
On Fri, Oct 20, 2017 at 5:53 PM, David Miller <davem@davemloft.net> wrote:
> From: David Miller <davem@davemloft.net>
> Date: Sat, 21 Oct 2017 01:50:08 +0100 (WEST)
>
>> From: Eric Dumazet <eric.dumazet@gmail.com>
>> Date: Wed, 18 Oct 2017 15:08:24 -0700
>>
>>> From: Eric Dumazet <edumazet@google.com>
>>>
>>> syzkaller got crashes in packet_getsockopt() processing
>>> PACKET_ROLLOVER_STATS command while another thread was managing
>>> to change po->rollover
>>>
>>> Using RCU will fix this bug. We might later add proper RCU annotations
>>> for sparse sake.
>>>
>>> Fixes: a9b6391814d5 ("packet: rollover statistics")
>>> Signed-off-by: Eric Dumazet <edumazet@google.com>
>>
>> Applied and queued up for -stable, but:
>>
>>> Willem de Bruijn <willemb@google.com>
>>
>> I didn't know what kind of tag you intended to use here for Willem
>> so just deleted the line.
>
> I just noticed 'v2' of this patch and replaced it in time before I
> pushed out :-)
Nice, thanks again !
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-10-21 1:11 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-18 22:08 [PATCH net] packet: avoid panic in packet_getsockopt() Eric Dumazet
2017-10-18 23:14 ` [PATCH v2 " Eric Dumazet
2017-10-20 19:11 ` kbuild test robot
2017-10-20 19:25 ` Eric Dumazet
2017-10-21 0:50 ` [PATCH " David Miller
2017-10-21 0:53 ` David Miller
2017-10-21 1:11 ` Eric Dumazet
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.