On Sun, Jul 10, 2016 at 7:42 AM, Sergei Trofimovich <[email protected]> wrote: > Hi netdev folk! > > Commit c14ac9451c34832554db33386a4393be8bba3a7b > broke pulseaudio (PA) over TCP.
Sorry that my patch broke your app and thanks for the bug report. Breaking PA was certainly not my intention. > PA does unusual thing: it calls > sendmsg(cmsg_type=SCM_CREDENTIALS) > > on a TCP socket. It's not a new PA behaviour though. > > Originally reported as PA bug (has more details) > https://bugs.freedesktop.org/show_bug.cgi?id=96873 > > It looks like kernel used to ignore control messages > but now it does not: > > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/diff/net/ipv4/tcp.c?id=c14ac9451c34832554db33386a4393be8bba3a7b > > + if (msg->msg_controllen) { > + err = sock_cmsg_send(sk, msg, &sockc); > + if (unlikely(err)) { > + err = -EINVAL; > + goto out_err; > + } > + } > > This change breaks streaming of pulse clients. > > Pulseaudio will be fixed at some point. > > But kernel change does not look like intentional > breakage of old behaviour. > > Perhaps kernel should have a grace period and only > warn about unsupported control messages for a socket? We have discussed ignoring certain control messages in another context: https://patchwork.ozlabs.org/patch/621837/ But the counter-argument (which I agree with) is that: we used to accept garbage in control messages before, but that doesn't mean we should give up on strict checking. This new problem is a bit different though. We always ignore control messages of other layers: ip_cmsg_send: if (cmsg->cmsg_level != SOL_IP) continue; sock_cmsg_send: if (cmsg->cmsg_level != SOL_SOCKET) continue; Semantically SCM_RIGHTS and SCM_CREDENTIALS belong to the SOL_UNIX layer but they are historically sent on SOL_SOCKET. I believe we should ignore them as we would if they were sent on SOL_UNIX: diff --git a/net/core/sock.c b/net/core/sock.c index 08bf97e..6239abf 100644 --- a/net/core/sock.c +++ b/net/core/sock.c @@ -1938,6 +1938,13 @@ int __sock_cmsg_send(struct sock *sk, struct msghdr *msg, struct cmsghdr *cmsg, sockc->tsflags &= ~SOF_TIMESTAMPING_TX_RECORD_MASK; sockc->tsflags |= tsflags; break; + /* SCM_RIGHTS and SCM_CREDENTIALS are semantically in SOL_UNIX + * yet they are sent on SOL_SOCKET. We should ignore them as + * we do for control messages not in the SOL_SOCKET layers. + */ + case SCM_RIGHTS: + case SCM_CREDENTIALS: + break; default: return -EINVAL; } David: Could you please let me know your thoughts? Thanks! Soheil > Last working kernel: v4.6 > > Thanks! > > -- > > Sergei
