From: Greg KH
> Sent: 13 September 2020 07:14
> On Sun, Sep 13, 2020 at 11:26:39AM +0530, Anant Thazhemadam wrote:
> > The crash report showed that there was a local variable;
> >
> > ----iovstack.i@__sys_sendmmsg created at:
> > ___sys_sendmsg net/socket.c:2388 [inline]
> > __sys_sendmmsg+0x6db/0xc90 net/socket.c:2480
> >
> > that was left uninitialized.
> >
> > The contents of iovstack are of interest, since the respective pointer
> > is passed down as an argument to sendmsg_copy_msghdr as well.
> > Initializing this contents of this stack prevents this bug from happening.
> >
> > Since the memory that was initialized is freed at the end of the function
> > call, memory leaks are not likely to be an issue.
> >
> > syzbot seems to have triggered this error by passing an array of 0's as
> > a parameter while making the initial system call.
> >
> > Reported-by: syzbot+09a5d591c1f98cf5e...@syzkaller.appspotmail.com
> > Tested-by: syzbot+09a5d591c1f98cf5e...@syzkaller.appspotmail.com
> > Signed-off-by: Anant Thazhemadam <anant.thazhema...@gmail.com>
> > ---
> > net/socket.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/net/socket.c b/net/socket.c
> > index 0c0144604f81..d74443dfd73b 100644
> > --- a/net/socket.c
> > +++ b/net/socket.c
> > @@ -2396,6 +2396,7 @@ static int ___sys_sendmsg(struct socket *sock, struct
> > user_msghdr __user *msg,
> > {
> > struct sockaddr_storage address;
> > struct iovec iovstack[UIO_FASTIOV], *iov = iovstack;
> > + memset(iov, 0, UIO_FASTIOV);
> > ssize_t err;
> >
> > msg_sys->msg_name = &address;
>
> I don't think you built this code change, otherwise you would have seen
> that it adds a build warning to the system, right?
Also it can't be the right 'fix' for whatever sysbot found.
(I can't find the sysbot report.)
Zeroing iov[] just slows down a path that is already too slow because
of the contorted functions used to read in iov[].
If it does need to be zerod then it would be needed in a lot
of other code paths that read in iov[].
If a zero length iov[] needs converting into a single entity
with a zero length - then that needs to be done elsewhere.
I've a patch series I might redo that changes the code that
reads in iov[] to return the address of any buffer that
needed to be malloced (more than UIV_FASTIO buffers) rather
than using the iov parameter to pass in the cache and
return the buffer to free.
It would be less confusing and error prone.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT,
UK
Registration No: 1397386 (Wales)
