Re: [PATCH v2 net 2/2] net: hamradio: scc: validate bufsize in SIOCSCCSMEM ioctl

[Joerg Reuter <jreuter@yaina.de>]

Looks great, thanks!

    73, Joerg

> The SIOCSCCSMEM ioctl copies a scc_mem_config from user space and
> assigns its bufsize field directly to scc->stat.bufsize without any
> range validation:
> 
>   scc->stat.bufsize = memcfg.bufsize;
> 
> If a privileged user (CAP_SYS_RAWIO) sets bufsize to 0, the receive
> interrupt handler later calls dev_alloc_skb(0) and immediately writes
> a KISS type byte via skb_put_u8() into a zero-capacity socket buffer,
> corrupting the adjacent skb_shared_info region.
> 
> Reject bufsize values smaller than 16; this is large enough to hold
> at least one KISS header byte plus useful data.
> 
> Cc: stable@vger.kernel.org
> Cc: linux-hams@vger.kernel.org
Acked-by: Joerg Reuter <jreuter@yaina.de>
> Signed-off-by: Mashiro Chen <mashiro.chen@mailbox.org>
> ---
>  drivers/net/hamradio/scc.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/hamradio/scc.c b/drivers/net/hamradio/scc.c
> index ae5048efde686a..8569db4a71401c 100644
> --- a/drivers/net/hamradio/scc.c
> +++ b/drivers/net/hamradio/scc.c
> @@ -1909,6 +1909,8 @@ static int scc_net_siocdevprivate(struct net_device *dev,
>  			if (!capable(CAP_SYS_RAWIO)) return -EPERM;
>  			if (!arg || copy_from_user(&memcfg, arg, sizeof(memcfg)))
>  				return -EINVAL;
> +			if (memcfg.bufsize < 16)
> +				return -EINVAL;
>  			scc->stat.bufsize   = memcfg.bufsize;
>  			return 0;
>  		
> -- 
> 2.53.0
> 

-- 
Joerg Reuter                                    http://yaina.de/jreuter
And I make my way to where the warm scent of soil fills the evening air. 
Everything is waiting quietly out there....                 (Anne Clark)