| Message ID | CAEQHRfAC9me4hGA+=+wcOpx+TAzqS723-kr_Y_Ej8dnWHp2fTw@mail.gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | Fix memory overwriting issue when copy an address to user space | expand |
From: lebon zhou > Sent: 20 July 2020 05:35 > To: davem@davemloft.net; kuba@kernel.org > Cc: linux-kernel@vger.kernel.org; netdev@vger.kernel.org > Subject: [PATCH] Fix memory overwriting issue when copy an address to user space > > When application provided buffer size less than sockaddr_storage, then > kernel will overwrite some memory area which may cause memory corruption, > e.g.: in recvmsg case, let msg_name=malloc(8) and msg_namelen=8, then > usually application can call recvmsg successful but actually application > memory get corrupted. Where? The copy_to_user() uses the short length provided by the user. There is even a comment saying that if the address is truncated the length returned to the user is the full length. Maybe the application is reusing the msg without re-initialising it properly. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
On Mon, Jul 20, 2020 at 11:12 PM David Laight <David.Laight@aculab.com> wrote: > > From: lebon zhou > > Sent: 20 July 2020 05:35 > > To: davem@davemloft.net; kuba@kernel.org > > Cc: linux-kernel@vger.kernel.org; netdev@vger.kernel.org > > Subject: [PATCH] Fix memory overwriting issue when copy an address to user space > > > > When application provided buffer size less than sockaddr_storage, then > > kernel will overwrite some memory area which may cause memory corruption, > > e.g.: in recvmsg case, let msg_name=malloc(8) and msg_namelen=8, then > > usually application can call recvmsg successful but actually application > > memory get corrupted. > > Where? > The copy_to_user() uses the short length provided by the user. > There is even a comment saying that if the address is truncated > the length returned to the user is the full length. > > Maybe the application is reusing the msg without re-initialising > it properly. It is not related with copy_to_user(), it is about move_addr_to_user() implementation itself, there is comment /*After copying the data up to the limit the user specifies...*/, but the fact is when (ulen < klen), this function will copy more content to user buffer over than user specifies in @ulen, this will cause the user buffer to corrupt, this patch fixes this issue.
diff --git a/net/socket.c b/net/socket.c index 976426d03f09..dc32b1b899df 100644 --- a/net/socket.c +++ b/net/socket.c @@ -229,7 +229,7 @@ static int move_addr_to_user(struct sockaddr_storage *kaddr, int klen, return err; if (len > klen) len = klen; - if (len < 0) + if (len < 0 || len < klen) return -EINVAL; if (len) {
When application provided buffer size less than sockaddr_storage, then kernel will overwrite some memory area which may cause memory corruption, e.g.: in recvmsg case, let msg_name=malloc(8) and msg_namelen=8, then usually application can call recvmsg successful but actually application memory get corrupted. Fix to return EINVAL when application buffer size less than sockaddr_storage. Signed-off-by: lebon.zhou <lebon.zhou@gmail.com> --- net/socket.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) if (audit_sockaddr(klen, kaddr)) -- 2.22.0