| Message ID | 20200124094144.15831-1-mpe@ellerman.id.au |
|---|---|
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
| Series | net: cxgb3_main: Add CAP_NET_ADMIN check to CHELSIO_GET_MEM | expand |
Hello! On 24.01.2020 12:41, Michael Ellerman wrote: > The cxgb3 driver for "Chelsio T3-based gigabit and 10Gb Ethernet > adapters" implements a custom ioctl as SIOCCHIOCTL/SIOCDEVPRIVATE in > cxgb_extension_ioctl(). > > One of the subcommands of the ioctl is CHELSIO_GET_MEM, which appears > to read memory directly out of the adapter and return it to userspace. > It's not entirely clear what the contents of the adapter memory > contains, but the assumption is that it shouldn't be accessible to all s/contains/is/? Else it sounds tautological. :-) > users. > > So add a CAP_NET_ADMIN check to the CHELSIO_GET_MEM case. Put it after > the is_offload() check, which matches two of the other subcommands in > the same function which also check for is_offload() and CAP_NET_ADMIN. > > Found by Ilja by code inspection, not tested as I don't have the > required hardware. > > Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com> > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> [...] MBR, Sergei
From: Michael Ellerman <mpe@ellerman.id.au> Date: Fri, 24 Jan 2020 20:41:44 +1100 > The cxgb3 driver for "Chelsio T3-based gigabit and 10Gb Ethernet > adapters" implements a custom ioctl as SIOCCHIOCTL/SIOCDEVPRIVATE in > cxgb_extension_ioctl(). > > One of the subcommands of the ioctl is CHELSIO_GET_MEM, which appears > to read memory directly out of the adapter and return it to userspace. > It's not entirely clear what the contents of the adapter memory > contains, but the assumption is that it shouldn't be accessible to all > users. > > So add a CAP_NET_ADMIN check to the CHELSIO_GET_MEM case. Put it after > the is_offload() check, which matches two of the other subcommands in > the same function which also check for is_offload() and CAP_NET_ADMIN. > > Found by Ilja by code inspection, not tested as I don't have the > required hardware. > > Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com> > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> Applied and queued up for -stable.
Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> writes: > Hello! > > On 24.01.2020 12:41, Michael Ellerman wrote: > >> The cxgb3 driver for "Chelsio T3-based gigabit and 10Gb Ethernet >> adapters" implements a custom ioctl as SIOCCHIOCTL/SIOCDEVPRIVATE in >> cxgb_extension_ioctl(). >> >> One of the subcommands of the ioctl is CHELSIO_GET_MEM, which appears >> to read memory directly out of the adapter and return it to userspace. >> It's not entirely clear what the contents of the adapter memory >> contains, but the assumption is that it shouldn't be accessible to all > > s/contains/is/? Else it sounds tautological. :-) Yeah you're right that would have been clearer. Dave beat me to it and has already applied it, but thanks for reviewing it anyway. cheers
diff --git a/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c b/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c index 58f89f6a040f..97ff8608f0ab 100644 --- a/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c +++ b/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c @@ -2448,6 +2448,8 @@ static int cxgb_extension_ioctl(struct net_device *dev, void __user *useraddr) if (!is_offload(adapter)) return -EOPNOTSUPP; + if (!capable(CAP_NET_ADMIN)) + return -EPERM; if (!(adapter->flags & FULL_INIT_DONE)) return -EIO; /* need the memory controllers */ if (copy_from_user(&t, useraddr, sizeof(t)))
The cxgb3 driver for "Chelsio T3-based gigabit and 10Gb Ethernet adapters" implements a custom ioctl as SIOCCHIOCTL/SIOCDEVPRIVATE in cxgb_extension_ioctl(). One of the subcommands of the ioctl is CHELSIO_GET_MEM, which appears to read memory directly out of the adapter and return it to userspace. It's not entirely clear what the contents of the adapter memory contains, but the assumption is that it shouldn't be accessible to all users. So add a CAP_NET_ADMIN check to the CHELSIO_GET_MEM case. Put it after the is_offload() check, which matches two of the other subcommands in the same function which also check for is_offload() and CAP_NET_ADMIN. Found by Ilja by code inspection, not tested as I don't have the required hardware. Reported-by: Ilja Van Sprundel <ivansprundel@ioactive.com> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> --- drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c | 2 ++ 1 file changed, 2 insertions(+)