Re: [PATCH v2 net-next] af_unix: dont send SCM_CREDENTIALS by default

5 messages, 4 authors, 2011-11-28 · open the first message on its own page

Re: [PATCH v2 net-next] af_unix: dont send SCM_CREDENTIALS by default

From: <hidden>
Date: 2011-09-20 02:12:40

On Mon, 19 Sep 2011 14:39:58 PDT, Tim Chen said:
Do we have to worry about the case where peer socket changes its flag
to SOCK_PASSCRED while packets are in flight?  If there isn't such
pathological use case, the patch looks fine to me.
I wouldn't think so - if you're sending a packet, and retroactively trying to
change the flag and expect it to work, your program is too ugly to live.  After
all, if the scheduler had cut off your timeslice and scheduledthe receiving
process before you set the flag, that packet would be delivered and done with
anyhow, and no amount of wishing will set that flag on an already-delivered
packet.

What *is* worth checking is that we DTRT if a process/thread is doing a send on
one CPU, and another process/thread with a shared file descriptor for that
socket is diddling the flag.  But if we just define it as "atomic op to change
the flag and other observers get whatever value their CPU sees at that
instant", I'm OK with that too.. ;)

Re: [PATCH v2 net-next] af_unix: dont send SCM_CREDENTIALS by default

From: Eric Dumazet <hidden>
Date: 2011-09-20 04:16:19

Le lundi 19 septembre 2011 à 22:10 -0400, Valdis.Kletnieks@vt.edu a
écrit :
On Mon, 19 Sep 2011 14:39:58 PDT, Tim Chen said:
quoted
Do we have to worry about the case where peer socket changes its flag
to SOCK_PASSCRED while packets are in flight?  If there isn't such
pathological use case, the patch looks fine to me.
I wouldn't think so - if you're sending a packet, and retroactively trying to
change the flag and expect it to work, your program is too ugly to live.  After
all, if the scheduler had cut off your timeslice and scheduledthe receiving
process before you set the flag, that packet would be delivered and done with
anyhow, and no amount of wishing will set that flag on an already-delivered
packet.

What *is* worth checking is that we DTRT if a process/thread is doing a send on
one CPU, and another process/thread with a shared file descriptor for that
socket is diddling the flag.  But if we just define it as "atomic op to change
the flag and other observers get whatever value their CPU sees at that
instant", I'm OK with that too.. ;)
Note : The man page does states :

"To receive a struct ucred message the SO_PASSCRED option  must  be
enabled  on  the socket."

But it doesnt say if the SO_PASSCRED option must be enabled before the
sender sends its message, or before receiver attempts to read it.

Once a message is queued on an unix socket, flipping SO_PASSCRED cant
change its content (adding or removing credentials), since sender might
already have disappeared.

So current code includes credentials in all sent messages, just in case
receiver actually fetch credentials.

There are probably programs that assume they can set SO_PASSCRED right
before calling recvmsg(). Are we taking risk to break them, or are we
gentle and provide a sysctl option to ease the transition, I dont
know...

Re: [PATCH v2 net-next] af_unix: dont send SCM_CREDENTIALS by default

From: tim <hidden>
Date: 2011-09-22 16:13:57

On Tue, 2011-09-20 at 06:16 +0200, Eric Dumazet wrote:
Le lundi 19 septembre 2011 à 22:10 -0400, Valdis.Kletnieks@vt.edu a
écrit :
quoted
On Mon, 19 Sep 2011 14:39:58 PDT, Tim Chen said:
quoted
Do we have to worry about the case where peer socket changes its flag
to SOCK_PASSCRED while packets are in flight?  If there isn't such
pathological use case, the patch looks fine to me.
I wouldn't think so - if you're sending a packet, and retroactively trying to
change the flag and expect it to work, your program is too ugly to live.  After
all, if the scheduler had cut off your timeslice and scheduledthe receiving
process before you set the flag, that packet would be delivered and done with
anyhow, and no amount of wishing will set that flag on an already-delivered
packet.

What *is* worth checking is that we DTRT if a process/thread is doing a send on
one CPU, and another process/thread with a shared file descriptor for that
socket is diddling the flag.  But if we just define it as "atomic op to change
the flag and other observers get whatever value their CPU sees at that
instant", I'm OK with that too.. ;)
Note : The man page does states :

"To receive a struct ucred message the SO_PASSCRED option  must  be
enabled  on  the socket."

But it doesnt say if the SO_PASSCRED option must be enabled before the
sender sends its message, or before receiver attempts to read it.

Once a message is queued on an unix socket, flipping SO_PASSCRED cant
change its content (adding or removing credentials), since sender might
already have disappeared.

So current code includes credentials in all sent messages, just in case
receiver actually fetch credentials.

There are probably programs that assume they can set SO_PASSCRED right
before calling recvmsg(). Are we taking risk to break them, or are we
gentle and provide a sysctl option to ease the transition, I dont
know...
Should we reconsider the original approach of reducing the
pid/credential references, with your fixes to correct its flaws in the
streaming msg case?

Tim

Re: [PATCH v2 net-next] af_unix: dont send SCM_CREDENTIALS by default

From: Michal Schmidt <hidden>
Date: 2011-11-28 13:23:32

On 09/20/2011 06:16 AM, Eric Dumazet wrote:
Note : The man page does states :

"To receive a struct ucred message the SO_PASSCRED option  must  be
enabled  on  the socket."

But it doesnt say if the SO_PASSCRED option must be enabled before the
sender sends its message, or before receiver attempts to read it.

Once a message is queued on an unix socket, flipping SO_PASSCRED cant
change its content (adding or removing credentials), since sender might
already have disappeared.

So current code includes credentials in all sent messages, just in case
receiver actually fetch credentials.

There are probably programs that assume they can set SO_PASSCRED right
before calling recvmsg(). Are we taking risk to break them, or are we
gentle and provide a sysctl option to ease the transition, I dont
know...
Such a case has just appeared:
https://bugzilla.redhat.com/show_bug.cgi?id=757628

systemd allows on-demand socket activation of services. It creates a listening socket without the SO_PASSCRED flag. When the first message arrives to the socket, systemd spawns the service and passes the socket's fd to it. The service sets SO_PASSCRED before actually receiving the message.

I can fix that in systemd, but there may be more cases like this.

Michal

Re: [PATCH v2 net-next] af_unix: dont send SCM_CREDENTIALS by default

From: Eric Dumazet <hidden>
Date: 2011-11-28 13:38:57

Le lundi 28 novembre 2011 à 14:23 +0100, Michal Schmidt a écrit :
On 09/20/2011 06:16 AM, Eric Dumazet wrote:
quoted
Note : The man page does states :

"To receive a struct ucred message the SO_PASSCRED option  must  be
enabled  on  the socket."

But it doesnt say if the SO_PASSCRED option must be enabled before the
sender sends its message, or before receiver attempts to read it.

Once a message is queued on an unix socket, flipping SO_PASSCRED cant
change its content (adding or removing credentials), since sender might
already have disappeared.

So current code includes credentials in all sent messages, just in case
receiver actually fetch credentials.

There are probably programs that assume they can set SO_PASSCRED right
before calling recvmsg(). Are we taking risk to break them, or are we
gentle and provide a sysctl option to ease the transition, I dont
know...
Such a case has just appeared:
https://bugzilla.redhat.com/show_bug.cgi?id=757628

systemd allows on-demand socket activation of services. It creates a 
listening socket without the SO_PASSCRED flag. When the first message 
arrives to the socket, systemd spawns the service and passes the 
socket's fd to it. The service sets SO_PASSCRED before actually 
receiving the message.

I can fix that in systemd, but there may be more cases like this.

Yes, we were afraid of this.

Performance drop is really huge and deserves some fixes in userland...

People add features to kernel (in this case namespaces) without thinking
on performance regression. So poor guys like us have to "fix" things
later, in a reasonable way.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help