* Re: Consequences of commit 16e5726269611b71c930054ffe9b858c1cea88eb
2012-01-12 20:59 ` Eric Dumazet
@ 2012-01-13 5:50 ` Eric Dumazet
2012-01-13 7:20 ` Bart Van Assche
1 sibling, 0 replies; 4+ messages in thread
From: Eric Dumazet @ 2012-01-13 5:50 UTC (permalink / raw)
To: Bart Van Assche; +Cc: netdev, David S. Miller, Mike Christie, Eric Paris
Le jeudi 12 janvier 2012 à 21:59 +0100, Eric Dumazet a écrit :
> Le jeudi 12 janvier 2012 à 19:14 +0000, Bart Van Assche a écrit :
> > Hi,
> >
> > If my analysis is correct commit
> > 16e5726269611b71c930054ffe9b858c1cea88eb ("af_unix: dont send
> > SCM_CREDENTIALS by default") changes the value of
> > NETLINK_CREDS(skb)->pid from the sender pid into zero. Does that mean
> > that the code using that construct did work in kernel 3.1 but that it
> > is broken in kernel 3.2 ? Should that commit be reverted or will
> > someone fix the code that uses NETLINK_CREDS() ? Would changing
> > NETLINK_CREDS(skb)->pid into NETLINK_CB(skb).pid be sufficient ?
> >
> > Thanks,
> >
> > Bart.
> >
> > $ git grep 'NETLINK_CREDS([a-zA-Z0-9_]*)->pid'
> > drivers/scsi/scsi_netlink.c: pid = NETLINK_CREDS(skb)->pid;
> > kernel/audit.c: pid = NETLINK_CREDS(skb)->pid;
>
>
> What is your problem exactly ?
>
>
So the underlying question is : should netlink_sendmsg() always include
credentials of the sender, or should the sender use the right API for
that.
If we include a default credential, we still allow the sender to
override it.
Probably netlink is not performance sensitive so following patch could
address the problem ?
I am still not sure its really needed.
Comments ?
diff --git a/include/net/scm.h b/include/net/scm.h
index d456f4c..4af5f90 100644
--- a/include/net/scm.h
+++ b/include/net/scm.h
@@ -71,9 +71,13 @@ static __inline__ void scm_destroy(struct scm_cookie *scm)
}
static __inline__ int scm_send(struct socket *sock, struct msghdr *msg,
- struct scm_cookie *scm)
+ struct scm_cookie *scm, bool populate)
{
memset(scm, 0, sizeof(*scm));
+
+ if (populate)
+ scm_set_cred(scm, task_tgid(current), current_cred());
+
unix_get_peersec_dgram(sock, scm);
if (msg->msg_controllen <= 0)
return 0;
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 629b061..c040277 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1323,7 +1323,7 @@ static int netlink_sendmsg(struct kiocb *kiocb, struct socket *sock,
if (NULL == siocb->scm)
siocb->scm = &scm;
- err = scm_send(sock, msg, siocb->scm);
+ err = scm_send(sock, msg, siocb->scm, true);
if (err < 0)
return err;
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index aad8fb6..f788eb9 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1438,7 +1438,7 @@ static int unix_dgram_sendmsg(struct kiocb *kiocb, struct socket *sock,
if (NULL == siocb->scm)
siocb->scm = &tmp_scm;
wait_for_unix_gc();
- err = scm_send(sock, msg, siocb->scm);
+ err = scm_send(sock, msg, siocb->scm, false);
if (err < 0)
return err;
@@ -1599,7 +1599,7 @@ static int unix_stream_sendmsg(struct kiocb *kiocb, struct socket *sock,
if (NULL == siocb->scm)
siocb->scm = &tmp_scm;
wait_for_unix_gc();
- err = scm_send(sock, msg, siocb->scm);
+ err = scm_send(sock, msg, siocb->scm, false);
if (err < 0)
return err;
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: Consequences of commit 16e5726269611b71c930054ffe9b858c1cea88eb
2012-01-12 20:59 ` Eric Dumazet
2012-01-13 5:50 ` Eric Dumazet
@ 2012-01-13 7:20 ` Bart Van Assche
1 sibling, 0 replies; 4+ messages in thread
From: Bart Van Assche @ 2012-01-13 7:20 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, David S. Miller, Mike Christie, Eric Paris
On Thu, Jan 12, 2012 at 9:59 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le jeudi 12 janvier 2012 à 19:14 +0000, Bart Van Assche a écrit :
> > If my analysis is correct commit
> > 16e5726269611b71c930054ffe9b858c1cea88eb ("af_unix: dont send
> > SCM_CREDENTIALS by default") changes the value of
> > NETLINK_CREDS(skb)->pid from the sender pid into zero. Does that mean
> > that the code using that construct did work in kernel 3.1 but that it
> > is broken in kernel 3.2 ? Should that commit be reverted or will
> > someone fix the code that uses NETLINK_CREDS() ? Would changing
> > NETLINK_CREDS(skb)->pid into NETLINK_CB(skb).pid be sufficient ?
> >
> > Thanks,
> >
> > Bart.
> >
> > $ git grep 'NETLINK_CREDS([a-zA-Z0-9_]*)->pid'
> > drivers/scsi/scsi_netlink.c: pid = NETLINK_CREDS(skb)->pid;
> > kernel/audit.c: pid = NETLINK_CREDS(skb)->pid;
>
> What is your problem exactly ?
While porting some kernel code to 3.2 that communicates with user
space via netlink and that works fine with kernel 3.1 and many
previous kernels I noticed that netlink communication from kernel to
user space was broken with kernel 3.2. I could get that code running
on kernel 3.2 after having reverted commit 16e5726. And with that
commit in place the code also started working if I changed a single
occurrence of NETLINK_CREDS(skb)->pid into NETLINK_CB(skb).pid in the
kernel code that processes received netlink messages.
Bart.
^ permalink raw reply [flat|nested] 4+ messages in thread