Messages in this thread | | | From | David Laight <> | Subject | RE: [PATCH] tun: make sure interface usage can not overflow | Date | Mon, 29 Sep 2014 11:04:00 +0000 |
| |
From: Kees Cook > This makes the size argument a const, since it is always populated by > the caller.
There is almost no point making parameters 'const. ('const foo *' makes sense).
> Additionally double-checks to make sure the copy_from_user > can never overflow, keeping CONFIG_DEBUG_STRICT_USER_COPY_CHECKS happy: > > In function 'copy_from_user', > inlined from '__tun_chr_ioctl' at drivers/net/tun.c:1871:7: > ... copy_from_user() buffer size is not provably correct
If 'ifreq_len' could be too big then you want to error the ioctl, not panic. If it can't be too big you don't need the check.
David
> Signed-off-by: Kees Cook <keescook@chromium.org> > --- > drivers/net/tun.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/tun.c b/drivers/net/tun.c > index acaaf6784179..a1f317cba206 100644 > --- a/drivers/net/tun.c > +++ b/drivers/net/tun.c > @@ -1855,7 +1855,7 @@ unlock: > } > > static long __tun_chr_ioctl(struct file *file, unsigned int cmd, > - unsigned long arg, int ifreq_len) > + unsigned long arg, const size_t ifreq_len) > { > struct tun_file *tfile = file->private_data; > struct tun_struct *tun; > @@ -1869,6 +1869,7 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd, > int ret; > > if (cmd == TUNSETIFF || cmd == TUNSETQUEUE || _IOC_TYPE(cmd) == 0x89) { > + BUG_ON(ifreq_len > sizeof(ifr)); > if (copy_from_user(&ifr, argp, ifreq_len)) > return -EFAULT; > } else { > -- > 1.9.1 > > > -- > Kees Cook > Chrome OS Security > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
| |