From patchwork Sun Jul 10 15:15:01 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Soheil Hassas Yeganeh X-Patchwork-Id: 646775 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3rnWyJ1VrXz9snm for ; Mon, 11 Jul 2016 01:15:52 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=google.com header.i=@google.com header.b=CRe/8ilr; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757207AbcGJPPt (ORCPT ); Sun, 10 Jul 2016 11:15:49 -0400 Received: from mail-oi0-f42.google.com ([209.85.218.42]:34048 "EHLO mail-oi0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757170AbcGJPPr (ORCPT ); Sun, 10 Jul 2016 11:15:47 -0400 Received: by mail-oi0-f42.google.com with SMTP id s66so118054274oif.1 for ; Sun, 10 Jul 2016 08:15:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=zNQ+bocnCW8Q0+5CEYDUCjI+cSCGtIn/u2eVra+/hGI=; b=CRe/8ilrBdbwBXa99vgLHfVRR4CThtg4739nufiUwfyePlH/Wh8UOZM7V+Lcj8OQq5 TeS6DyZwT2PWBCwWTisxoDpVC0P0fpIQqCM/ZZsATnIBqhvZUBI3k4T7M7g82sPj3fD1 a/VgjKW5C4z8jyt/qQ8AIsmZ9MdqksW4qkT/t0hPDPT3I+yn6zTS2WJhZ4LzydWs63qB edDMlKbJMYxFKFYcYUt8Oigx0UOFNsYCe4qtFIqe/20kvBJuJQj/hLOllUUA9AWbEKiT eUnIHybbEqPn1nbztWXEhKxT/+FpSoZ2Ls5v7QLJEM+x+e0AQli17z9Oc2QWSqJAHrxd NwWw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=zNQ+bocnCW8Q0+5CEYDUCjI+cSCGtIn/u2eVra+/hGI=; b=JhtRPwjJWnB4L/c82+ZLlpERqYDMxcy2J9es0O/Fa91dNMYbJ+g7igCCZJo3soZTtm LNIbZ94fCZnyKJs4GVwtS6wg69+Te2g4v4dyAQvG/H6PxP7kbElWCudBuSZx3+yzA3/6 Xad9cxsB+KjuGspkOFvLj5jHN+JNDScJL32DhiYNlXju79aQmh49tF8p3Jyqe+90hcVm NBsivWXQ8zfXOs4AfKtf6JSIf1bXv6HaLkZNTMPAkGKmdNU+h9kb5LJTCHTXhC0z+Amb EjSvspA//PC6A9JCi1xgFnGlQuExkxuzoMr/YDuPwR4tVd/FhuyUFbQtxkyZKlDv/4tJ 0Bgw== X-Gm-Message-State: ALyK8tJEwUGE2cnaymWcdXTQcUjaAqzz7JATMNRJzY79IEWo9wBcDDOu6mC5LbZGBT73SxE9cdjLfDPT6B1J/OKq X-Received: by 10.157.62.132 with SMTP id b4mr8525746otc.178.1468163740887; Sun, 10 Jul 2016 08:15:40 -0700 (PDT) MIME-Version: 1.0 Received: by 10.157.32.201 with HTTP; Sun, 10 Jul 2016 08:15:01 -0700 (PDT) In-Reply-To: <20160710124240.7d484f44@sf> References: <20160710124240.7d484f44@sf> From: Soheil Hassas Yeganeh Date: Sun, 10 Jul 2016 11:15:01 -0400 Message-ID: Subject: Re: [BUG?] tcp regression in v4.7-r1: c14ac9451c34832554db33386a4393be8bba3a7b breaks pulseaudio over TCP To: Sergei Trofimovich , "David S. Miller" Cc: netdev , Willem de Bruijn , Tanu Kaskinen Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Sun, Jul 10, 2016 at 7:42 AM, Sergei Trofimovich 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: } David: Could you please let me know your thoughts? Thanks! Soheil > Last working kernel: v4.6 > > Thanks! > > -- > > Sergei 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;