* Consequences of commit 16e5726269611b71c930054ffe9b858c1cea88eb
@ 2012-01-12 19:14 Bart Van Assche
2012-01-12 20:59 ` Eric Dumazet
0 siblings, 1 reply; 4+ messages in thread
From: Bart Van Assche @ 2012-01-12 19:14 UTC (permalink / raw)
To: netdev; +Cc: Eric Dumazet, David S. Miller, Mike Christie, Eric Paris
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;
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Consequences of commit 16e5726269611b71c930054ffe9b858c1cea88eb
2012-01-12 19:14 Consequences of commit 16e5726269611b71c930054ffe9b858c1cea88eb Bart Van Assche
@ 2012-01-12 20:59 ` Eric Dumazet
2012-01-13 5:50 ` Eric Dumazet
2012-01-13 7:20 ` Bart Van Assche
0 siblings, 2 replies; 4+ messages in thread
From: Eric Dumazet @ 2012-01-12 20:59 UTC (permalink / raw)
To: Bart Van Assche; +Cc: netdev, David S. Miller, Mike Christie, Eric Paris
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 ?
^ permalink raw reply [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: 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
end of thread, other threads:[~2012-01-13 7:20 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-01-12 19:14 Consequences of commit 16e5726269611b71c930054ffe9b858c1cea88eb Bart Van Assche
2012-01-12 20:59 ` Eric Dumazet
2012-01-13 5:50 ` Eric Dumazet
2012-01-13 7:20 ` Bart Van Assche
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox