In reviewing the network code I found a number of places where an error exit
would fail to free an skb obtained from a skb_recv_datagram() call. The errors
in question involve faults when copying iovecs, and so are probably rare but
could possibly be generated intentionally. The attached patch should fix the
leaks.
Also included is a change for ipv6 multicasting, where a cloned skb was being
queued on the wrong sk (and the allocation wasn't being checked.) I also added
code to lock the hashes following the ipv4 practice.
I'm running with the ipv4 and af_packet changes, but haven't tested the other
subsystems.
Regards,
Bill
--------------6B78ED3B0BA113C66B0A7EB4
Content-Type: text/plain; charset=us-ascii; name="net_skbleak88-patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline; filename="net_skbleak88-patch"
--- linux-2.1.88/net/ipv4/udp.c.old Sat Feb 21 09:25:04 1998
+++ linux-2.1.88/net/ipv4/udp.c Fri Mar 6 18:52:30 1998
@@ -816,8 +816,10 @@
struct sk_buff *skb;
unsigned long amount;
- if (sk->state == TCP_LISTEN) return(-EINVAL);
+ if (sk->state == TCP_LISTEN)
+ return(-EINVAL);
amount = 0;
+ /* N.B. Is this interrupt safe?? */
skb = skb_peek(&sk->receive_queue);
if (skb != NULL) {
/*
@@ -843,13 +845,11 @@
*/
int udp_recvmsg(struct sock *sk, struct msghdr *msg, int len,
- int noblock, int flags,int *addr_len)
+ int noblock, int flags, int *addr_len)
{
- int copied = 0;
- int truesize;
+ struct sockaddr_in *sin = (struct sockaddr_in *)msg->msg_name;
struct sk_buff *skb;
- int er;
- struct sockaddr_in *sin=(struct sockaddr_in *)msg->msg_name;
+ int copied, err;
/*
* Check any passed addresses
@@ -859,14 +859,12 @@
*addr_len=sizeof(*sin);
if (sk->ip_recverr && (skb = skb_dequeue(&sk->error_queue)) != NULL) {
- er = sock_error(sk);
- if (msg->msg_controllen == 0) {
- skb_free_datagram(sk, skb);
- return er;
+ err = sock_error(sk);
+ if (msg->msg_controllen != 0) {
+ put_cmsg(msg, SOL_IP, IP_RECVERR, skb->len, skb->data);
+ err = 0;
}
- put_cmsg(msg, SOL_IP, IP_RECVERR, skb->len, skb->data);
- skb_free_datagram(sk, skb);
- return 0;
+ goto out_free;
}
/*
@@ -874,25 +872,25 @@
* the finished NET3, it will do _ALL_ the work!
*/
- skb=skb_recv_datagram(sk,flags,noblock,&er);
- if(skb==NULL)
- return er;
+ skb = skb_recv_datagram(sk, flags, noblock, &err);
+ if (!skb)
+ goto out;
- truesize = skb->len - sizeof(struct udphdr);
- copied = truesize;
- if (len < truesize)
+ copied = skb->len - sizeof(struct udphdr);
+ if (copied > len)
{
- msg->msg_flags |= MSG_TRUNC;
copied = len;
+ msg->msg_flags |= MSG_TRUNC;
}
/*
* FIXME : should use udp header size info value
*/
- er = skb_copy_datagram_iovec(skb,sizeof(struct udphdr),msg->msg_iov,copied);
- if (er)
- return er;
+ err = skb_copy_datagram_iovec(skb, sizeof(struct udphdr), msg->msg_iov,
+ copied);
+ if (err)
+ goto out_free;
sk->stamp=skb->stamp;
/* Copy the address. */
@@ -921,9 +919,12 @@
}
if (sk->ip_cmsg_flags)
ip_cmsg_recv(msg, skb);
+ err = copied;
+out_free:
skb_free_datagram(sk, skb);
- return(copied);
+out:
+ return err;
}
int udp_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len)
--- linux-2.1.88/net/ipv6/udp.c.old Wed Feb 11 00:06:15 1998
+++ linux-2.1.88/net/ipv6/udp.c Fri Mar 6 18:51:56 1998
@@ -300,10 +300,8 @@
int udpv6_recvmsg(struct sock *sk, struct msghdr *msg, int len,
int noblock, int flags, int *addr_len)
{
- int copied = 0;
- int truesize;
struct sk_buff *skb;
- int err;
+ int copied, err;
/*
* Check any passed addresses
@@ -318,16 +316,13 @@
*/
skb = skb_recv_datagram(sk, flags, noblock, &err);
- if(skb==NULL)
- return err;
+ if (!skb)
+ goto out;
- truesize=ntohs(((struct udphdr *)skb->h.raw)->len) - sizeof(struct udphdr);
-
- copied=truesize;
-
- if(copied>len) {
- copied=len;
- msg->msg_flags|=MSG_TRUNC;
+ copied = ntohs(((struct udphdr *)skb->h.raw)->len) - sizeof(struct udphdr);
+ if (copied > len) {
+ copied = len;
+ msg->msg_flags |= MSG_TRUNC;
}
/*
@@ -337,7 +332,7 @@
err = skb_copy_datagram_iovec(skb, sizeof(struct udphdr),
msg->msg_iov, copied);
if (err)
- return err;
+ goto out_free;
sk->stamp=skb->stamp;
@@ -346,7 +341,6 @@
struct sockaddr_in6 *sin6;
sin6 = (struct sockaddr_in6 *) msg->msg_name;
-
sin6->sin6_family = AF_INET6;
sin6->sin6_port = skb->h.uh->source;
@@ -361,9 +355,12 @@
datagram_recv_ctl(sk, msg, skb);
}
}
-
+ err = copied;
+
+out_free:
skb_free_datagram(sk, skb);
- return(copied);
+out:
+ return err;
}
void udpv6_err(int type, int code, unsigned char *buff, __u32 info,
@@ -457,6 +454,7 @@
{
struct sock *sk, *sk2;
+ SOCKHASH_LOCK();
sk = udp_hash[ntohs(uh->dest) & (UDP_HTABLE_SIZE - 1)];
sk = udp_v6_mcast_next(sk, uh->dest, daddr, uh->source, saddr);
if(sk) {
@@ -465,7 +463,7 @@
uh->dest, saddr,
uh->source, daddr))) {
struct sk_buff *buff = skb_clone(skb, GFP_ATOMIC);
- if(sock_queue_rcv_skb(sk, buff) < 0) {
+ if (buff && sock_queue_rcv_skb(sk2, buff) < 0) {
buff->sk = NULL;
kfree_skb(buff);
}
@@ -475,6 +473,7 @@
skb->sk = NULL;
kfree_skb(skb);
}
+ SOCKHASH_UNLOCK();
}
int udpv6_rcv(struct sk_buff *skb, struct device *dev,
--- linux-2.1.88/net/ipv6/raw.c.old Wed Feb 11 00:06:14 1998
+++ linux-2.1.88/net/ipv6/raw.c Fri Mar 6 18:54:45 1998
@@ -236,13 +236,11 @@
*/
int rawv6_recvmsg(struct sock *sk, struct msghdr *msg, int len,
- int noblock, int flags,int *addr_len)
+ int noblock, int flags, int *addr_len)
{
- struct sockaddr_in6 *sin6=(struct sockaddr_in6 *)msg->msg_name;
+ struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)msg->msg_name;
struct sk_buff *skb;
- int copied=0;
- int err;
-
+ int copied, err;
if (flags & MSG_OOB)
return -EOPNOTSUPP;
@@ -253,32 +251,32 @@
if (addr_len)
*addr_len=sizeof(*sin6);
- skb=skb_recv_datagram(sk, flags, noblock, &err);
- if(skb==NULL)
- return err;
+ skb = skb_recv_datagram(sk, flags, noblock, &err);
+ if (!skb)
+ goto out;
copied = min(len, skb->tail - skb->h.raw);
err = skb_copy_datagram_iovec(skb, 0, msg->msg_iov, copied);
sk->stamp=skb->stamp;
-
if (err)
- return err;
+ goto out_free;
/* Copy the address. */
if (sin6) {
sin6->sin6_family = AF_INET6;
memcpy(&sin6->sin6_addr, &skb->nh.ipv6h->saddr,
sizeof(struct in6_addr));
-
- *addr_len = sizeof(struct sockaddr_in6);
}
if (msg->msg_controllen)
datagram_recv_ctl(sk, msg, skb);
+ err = copied;
+out_free:
skb_free_datagram(sk, skb);
- return (copied);
+out:
+ return err;
}
/*
--- linux-2.1.88/net/unix/af_unix.c.old Wed Feb 11 00:06:15 1998
+++ linux-2.1.88/net/unix/af_unix.c Fri Mar 6 16:04:08 1998
@@ -1033,8 +1033,9 @@
size=len-sent;
- if (size>(sk->sndbuf-sizeof(struct sk_buff))/2) /* Keep two messages in the pipe so it schedules better */
- size=(sk->sndbuf-sizeof(struct sk_buff))/2;
+ /* Keep two messages in the pipe so it schedules better */
+ if (size > (sk->sndbuf - sizeof(struct sk_buff)) / 2)
+ size = (sk->sndbuf - sizeof(struct sk_buff)) / 2;
/*
* Keep to page sized kmalloc()'s as various people
@@ -1056,7 +1057,7 @@
if (skb==NULL)
{
if (sent)
- return sent;
+ goto out;
return err;
}
@@ -1074,6 +1075,7 @@
if (scm->fp)
unix_attach_fds(scm, skb);
+ /* N.B. this could fail with -EFAULT */
memcpy_fromiovec(skb_put(skb,size), msg->msg_iov, size);
other=unix_peer(sk);
@@ -1082,7 +1084,7 @@
{
kfree_skb(skb);
if(sent)
- return sent;
+ goto out;
send_sig(SIGPIPE,current,0);
return -EPIPE;
}
@@ -1091,6 +1093,7 @@
other->data_ready(other,size);
sent+=size;
}
+out:
return sent;
}
@@ -1121,20 +1124,20 @@
msg->msg_namelen = 0;
- skb=skb_recv_datagram(sk, flags, noblock, &err);
- if(skb==NULL)
- return err;
+ skb = skb_recv_datagram(sk, flags, noblock, &err);
+ if (!skb)
+ goto out;
if (msg->msg_name)
{
+ msg->msg_namelen = sizeof(short);
if (skb->sk->protinfo.af_unix.addr)
{
- memcpy(msg->msg_name, skb->sk->protinfo.af_unix.addr->name,
- skb->sk->protinfo.af_unix.addr->len);
msg->msg_namelen=skb->sk->protinfo.af_unix.addr->len;
+ memcpy(msg->msg_name,
+ skb->sk->protinfo.af_unix.addr->name,
+ skb->sk->protinfo.af_unix.addr->len);
}
- else
- msg->msg_namelen=sizeof(short);
}
if (size > skb->len)
@@ -1142,8 +1145,9 @@
else if (size < skb->len)
msg->msg_flags |= MSG_TRUNC;
- if (skb_copy_datagram_iovec(skb, 0, msg->msg_iov, size))
- return -EFAULT;
+ err = skb_copy_datagram_iovec(skb, 0, msg->msg_iov, size);
+ if (err)
+ goto out_free;
scm->creds = *UNIXCREDS(skb);
@@ -1169,8 +1173,12 @@
if (UNIXCB(skb).fp)
scm->fp = scm_fp_dup(UNIXCB(skb).fp);
}
+ err = size;
+
+out_free:
skb_free_datagram(sk,skb);
- return size;
+out:
+ return err;
}
@@ -1189,7 +1197,7 @@
if (flags&MSG_OOB)
return -EOPNOTSUPP;
- if(flags&MSG_WAITALL)
+ if (flags&MSG_WAITALL)
target = size;
@@ -1245,18 +1253,19 @@
/* Copy address just once */
if (sunaddr)
{
+ msg->msg_namelen = sizeof(short);
if (skb->sk->protinfo.af_unix.addr)
{
- memcpy(sunaddr, skb->sk->protinfo.af_unix.addr->name,
- skb->sk->protinfo.af_unix.addr->len);
msg->msg_namelen=skb->sk->protinfo.af_unix.addr->len;
+ memcpy(sunaddr,
+ skb->sk->protinfo.af_unix.addr->name,
+ skb->sk->protinfo.af_unix.addr->len);
}
- else
- msg->msg_namelen=sizeof(short);
sunaddr = NULL;
}
chunk = min(skb->len, size);
+ /* N.B. This could fail with -EFAULT */
memcpy_toiovec(msg->msg_iov, skb->data, chunk);
copied += chunk;
size -= chunk;
--- linux-2.1.88/net/packet/af_packet.c.old Wed Feb 11 00:06:15 1998
+++ linux-2.1.88/net/packet/af_packet.c Fri Mar 6 18:56:56 1998
@@ -778,9 +778,8 @@
int flags, struct scm_cookie *scm)
{
struct sock *sk = sock->sk;
- int copied=0;
struct sk_buff *skb;
- int err;
+ int copied, err;
#if 0
/* What error should we return now? EUNATTACH? */
@@ -816,7 +815,7 @@
*/
if(skb==NULL)
- return err;
+ goto out;
/*
* You lose any data beyond the buffer you gave. If it worries a
@@ -824,7 +823,7 @@
*/
copied = skb->len;
- if(copied>len)
+ if (copied > len)
{
copied=len;
msg->msg_flags|=MSG_TRUNC;
@@ -833,9 +832,7 @@
/* We can't use skb_copy_datagram here */
err = memcpy_toiovec(msg->msg_iov, skb->data, copied);
if (err)
- {
- return -EFAULT;
- }
+ goto out_free;
sk->stamp=skb->stamp;
@@ -843,13 +840,15 @@
memcpy(msg->msg_name, skb->cb, msg->msg_namelen);
/*
- * Free or return the buffer as appropriate. Again this hides all the
- * races and re-entrancy issues from us.
+ * Free or return the buffer as appropriate. Again this
+ * hides all the races and re-entrancy issues from us.
*/
+ err = copied;
+out_free:
skb_free_datagram(sk, skb);
-
- return(copied);
+out:
+ return err;
}
#ifdef CONFIG_SOCK_PACKET
--- linux-2.1.88/net/ipx/af_ipx.c.old Sat Feb 21 09:25:04 1998
+++ linux-2.1.88/net/ipx/af_ipx.c Fri Mar 6 19:06:15 1998
@@ -998,7 +998,8 @@
return -EPROTONOSUPPORT;
dev=dev_get(idef->ipx_device);
- if(dev==NULL) return -ENODEV;
+ if (dev==NULL)
+ return -ENODEV;
intrfc = ipxitf_find_using_phys(dev, dlink_type);
if (intrfc != NULL) {
@@ -1107,9 +1108,9 @@
sipx->sipx_family=AF_IPX;
sipx->sipx_network=ipxif->if_netnum;
memcpy(sipx->sipx_node, ipxif->if_node, sizeof(sipx->sipx_node));
- err = copy_to_user(arg,&ifr,sizeof(ifr));
- if (err)
- return -EFAULT;
+ err = -EFAULT;
+ if (!copy_to_user(arg, &ifr, sizeof(ifr)))
+ err = 0;
return err;
}
case SIOCAIPXITFCRT:
@@ -2134,32 +2135,28 @@
struct sock *sk=sock->sk;
struct sockaddr_ipx *sipx=(struct sockaddr_ipx *)msg->msg_name;
struct ipxhdr *ipx = NULL;
- int copied = 0;
- int truesize;
struct sk_buff *skb;
- int err;
+ int copied, err;
if (sk->zapped)
return -ENOTCONN;
skb=skb_recv_datagram(sk,flags&~MSG_DONTWAIT,flags&MSG_DONTWAIT,&err);
- if(skb==NULL)
- return err;
+ if (!skb)
+ goto out;
ipx = skb->nh.ipxh;
- truesize=ntohs(ipx->ipx_pktsize) - sizeof(struct ipxhdr);
-
- copied = truesize;
+ copied = ntohs(ipx->ipx_pktsize) - sizeof(struct ipxhdr);
if(copied > size)
{
copied=size;
msg->msg_flags|=MSG_TRUNC;
}
- err = skb_copy_datagram_iovec(skb,sizeof(struct ipxhdr),msg->msg_iov,copied);
-
+ err = skb_copy_datagram_iovec(skb, sizeof(struct ipxhdr), msg->msg_iov,
+ copied);
if (err)
- return err;
+ goto out_free;
msg->msg_namelen = sizeof(*sipx);
@@ -2171,9 +2168,12 @@
sipx->sipx_network=ipx->ipx_source.net;
sipx->sipx_type = ipx->ipx_type;
}
- skb_free_datagram(sk, skb);
+ err = copied;
- return(copied);
+out_free:
+ skb_free_datagram(sk, skb);
+out:
+ return err;
}
/*
@@ -2228,11 +2228,12 @@
{
if(sk->stamp.tv_sec==0)
return -ENOENT;
- ret = copy_to_user((void *)arg,&sk->stamp,sizeof(struct timeval));
- if (ret)
- ret = -EFAULT;
+ ret = -EFAULT;
+ if (!copy_to_user((void *)arg, &sk->stamp,
+ sizeof(struct timeval)))
+ ret = 0;
}
- return 0;
+ return ret;
}
case SIOCGIFDSTADDR:
case SIOCSIFDSTADDR:
--------------6B78ED3B0BA113C66B0A7EB4--
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu