Skip to contentSkip to conversation

netrom: fix invalid use of sizeof in nr_recvmsg()

PATCH f6bvp@free 2 participants 2 messages Apr 21, 2013

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

f6bvp@free

author
#0 lore raw

Hi,

According to the proximity of NetRom and Rose codes I looked at af_rose.c and it seems that similarly sockaddr_rose structure is let uninitialized in rose_recvmsg().

Then, would you consider the following patch interesting to be committed ?

net/rose/af_rose.c 2013-04-21 17:26:06.914967897 +0200 renamed from net/rose/af_rose.c 2013-04-17 07:11:28.000000000 +0200 +1 -0
@@ -1257,6 +1257,7 @@ static int rose_recvmsg(struct kiocb *io
        skb_copy_datagram_iovec(skb, 0, msg->msg_iov, copied);
 
        if (srose != NULL) {
+               memset(srose, 0, sizeof(*srose));
                srose->srose_family = AF_ROSE;
                srose->srose_addr   = rose->dest_addr;
                srose->srose_call   = rose->dest_call;

Bernard Pidoux

#1 lore raw
Quoted 12 lines · On Sun, Apr 21, 2013 at 7:56 PM, f6bvp@free <[email protected]> wrote:

Hi,

According to the proximity of NetRom and Rose codes I looked at af_rose.c and it seems that similarly sockaddr_rose structure is let uninitialized in rose_recvmsg().

Then, would you consider the following patch interesting to be committed ?

net/rose/af_rose.c 2013-04-21 17:26:06.914967897 +0200 static int rose_recvmsg(struct kiocb *io · line 1263

Quoted 7 lines
net/rose/af_rose.c 2013-04-21 17:26:06.914967897 +0200 renamed from net/rose/af_rose.c 2013-04-17 07:11:28.000000000 +0200 +1 -0
@@ -1257,6 +1257,7 @@ static int rose_recvmsg(struct kiocb *io
        skb_copy_datagram_iovec(skb, 0, msg->msg_iov, copied);
 
        if (srose != NULL) {
+               memset(srose, 0, sizeof(*srose));
                srose->srose_family = AF_ROSE;
                srose->srose_addr   = rose->dest_addr;
                srose->srose_call   = rose->dest_call;

Thanks, but something more complete is already in Linus tree (sizeof(*srose) is not enough):

commit 4a184233f21645cf0b719366210ed445d1024d72

Author: Mathias Krause <[email protected]>
Date:   Sun Apr 7 01:51:59 2013 +0000

rose: fix info leak via msg_name in rose_recvmsg()

The code in rose_recvmsg() does not initialize all of the members of struct sockaddr_rose/full_sockaddr_rose when filling the sockaddr info. Nor does it initialize the padding bytes of the structure inserted by the compiler for alignment. This will lead to leaking uninitialized kernel stack bytes in net/socket.c.

Fix the issue by initializing the memory used for sockaddr info with memset(0).

Cc Ralf Baechle <[email protected]>
Signed-off-by Mathias Krause <[email protected]>
Signed-off-by David S. Miller <[email protected]>
net/rose/af_rose.c +1 -0
index cf68e6e..9c83474 100644
@@ -1253,6 +1253,7 @@ static int rose_recvmsg(struct kiocb *iocb, struct socket
        skb_copy_datagram_iovec(skb, 0, msg->msg_iov, copied);
 
        if (srose != NULL) {
+               memset(srose, 0, msg->msg_namelen);
                srose->srose_family = AF_ROSE;
                srose->srose_addr   = rose->dest_addr;
                srose->srose_call   = rose->dest_call;