Re: [PATCH][unix] missing barriers in some of unix_sock ->addr and ->path accesses

Re: [PATCH][unix] missing barriers in some of unix_sock ->addr and ->path accesses

[Sasha Levin]

Hi,

[This is an automated email]

This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: .+

The bot has tested the following trees: v4.20.10, v4.19.23, v4.14.101, v4.9.158, v4.4.174, v3.18.134.

v4.20.10: Build OK!
v4.19.23: Build OK!
v4.14.101: Failed to apply! Possible dependencies:
    27204aaa9dc6 ("tcp: uniform the set up of sockets after successful connection")
    3830d0771ef6 ("soc: qcom: Introduce QMI helpers")
    64ac5f5977df ("tipc: refactor function filter_rcv()")
    75da2163dbb6 ("tipc: introduce communication groups")
    9b2c45d479d0 ("net: make getname() functions return length rather than use int* parameter")
    9b8a11e82615 ("soc: qcom: Introduce QMI encoder/decoder")
    a3f8683bf7d5 ("->poll() methods should return __poll_t")
    a50ff19d0658 ("of/platform: Generalize /reserved-memory handling")
    a80ae5306a73 ("tipc: improve destination linked list")
    ade994f4f6c8 ("net: annotate ->poll() instances")
    d1de6d6c639b ("soc: qcom: Remote filesystem memory driver")

v4.9.158: Failed to apply! Possible dependencies:
    13d3b1ebe287 ("bpf: Support for setting initial receive window")
    27204aaa9dc6 ("tcp: uniform the set up of sockets after successful connection")
    3007098494be ("cgroup: add support for eBPF programs")
    3830d0771ef6 ("soc: qcom: Introduce QMI helpers")
    40304b2a1567 ("bpf: BPF support for sock_ops")
    526735ddc0ae ("net: fix AF_SMC related typo")
    610236587600 ("bpf: Add new cgroup attach type to enable sock modifications")
    8550f328f45d ("bpf: Support for per connection SYN/SYN-ACK RTOs")
    8c9814b97002 ("net: convert unix_address.refcnt from atomic_t to refcount_t")
    9872a4bde31b ("bpf: Add TCP connection BPF callbacks")
    9b2c45d479d0 ("net: make getname() functions return length rather than use int* parameter")
    9b8a11e82615 ("soc: qcom: Introduce QMI encoder/decoder")
    a3f8683bf7d5 ("->poll() methods should return __poll_t")
    a50ff19d0658 ("of/platform: Generalize /reserved-memory handling")
    ac7138746e14 ("smc: establish new socket family")
    ade994f4f6c8 ("net: annotate ->poll() instances")
    b2cd12574aa3 ("bpf: Refactor cgroups code in prep for new type")
    cb5da847af44 ("tipc: set kern=0 in sk_alloc() during tipc_accept()")
    cdfbabfb2f0c ("net: Work around lockdep limitation in sockets that use sockets")
    d1de6d6c639b ("soc: qcom: Remote filesystem memory driver")

v4.4.174: Failed to apply! Possible dependencies:
    35da60941e44 ("pstore/ram: add Device Tree bindings")
    3830d0771ef6 ("soc: qcom: Introduce QMI helpers")
    4420cfd3f51c ("staging: lustre: format properly all comment blocks for LNet core")
    44a7185c2ae6 ("of/platform: Add common method to populate default bus")
    4c3523623dc0 ("net: add driver for Netronome NFP4000/NFP6000 NIC VFs")
    529182e204db ("ramoops: use DT reserved-memory bindings")
    53692b1de419 ("sctp: Rename NETIF_F_SCTP_CSUM to NETIF_F_SCTP_CRC")
    5fd88337d209 ("staging: lustre: fix all conditional comparison to zero in LNet layer")
    8c9814b97002 ("net: convert unix_address.refcnt from atomic_t to refcount_t")
    9b2c45d479d0 ("net: make getname() functions return length rather than use int* parameter")
    9b8a11e82615 ("soc: qcom: Introduce QMI encoder/decoder")
    a188222b6ed2 ("net: Rename NETIF_F_ALL_CSUM to NETIF_F_CSUM_MASK")
    a50ff19d0658 ("of/platform: Generalize /reserved-memory handling")
    d1de6d6c639b ("soc: qcom: Remote filesystem memory driver")
    fc520f8b4fa3 ("of/platform: disable the of_platform_default_populate_init() for all the ppc boards")

v3.18.134: Failed to apply! Possible dependencies:
    35da60941e44 ("pstore/ram: add Device Tree bindings")
    3830d0771ef6 ("soc: qcom: Introduce QMI helpers")
    43443ad692cf ("of/platform: add function to populate default bus")
    44a7185c2ae6 ("of/platform: Add common method to populate default bus")
    4b638df4c9d5 ("soc: qcom: Add Shared Memory Manager driver")
    529182e204db ("ramoops: use DT reserved-memory bindings")
    7ce75bb2c05e ("ARM: qcom: Add Subsystem Power Manager (SPM) driver")
    7d0c8beea6b8 ("soc: qcom: Reorder SMEM/SMD configs")
    8c9814b97002 ("net: convert unix_address.refcnt from atomic_t to refcount_t")
    936f14cf4e67 ("soc: qcom: Driver for the Qualcomm RPM over SMD")
    9b2c45d479d0 ("net: make getname() functions return length rather than use int* parameter")
    9b8a11e82615 ("soc: qcom: Introduce QMI encoder/decoder")
    a50ff19d0658 ("of/platform: Generalize /reserved-memory handling")
    d1de6d6c639b ("soc: qcom: Remote filesystem memory driver")
    f2ab3298fb49 ("soc: qcom: Add Shared Memory Driver")
    fc520f8b4fa3 ("of/platform: disable the of_platform_default_populate_init() for all the ppc boards")


How should we proceed with this patch?

--
Thanks,
Sasha

Re: [PATCH][unix] missing barriers in some of unix_sock ->addr and ->path accesses

[Al Viro]

On Mon, Feb 18, 2019 at 09:14:33PM +0000, Sasha Levin wrote:
> Hi,
> 
> [This is an automated email]
> 
> This commit has been processed because it contains a -stable tag.

Ugh...  Should've removed Cc; stable from netdev posting; my apologies.

> How should we proceed with this patch?

Wait for it to get into davem's tree, for starters?

Sorry about that, again...

FWIW, further adventures in net/unix land:

unix_dgram_poll() contains
                /* connection hasn't started yet? */
                if (sk->sk_state == TCP_SYN_SENT)
                        return mask;
and nothing in there sets TCP_SYN_SENT state (not that it would've made
any sense of AF_UNIX).

unix_poll() contains
        /* Connection-based need to check for termination and startup */
        if ((sk->sk_type == SOCK_STREAM || sk->sk_type == SOCK_SEQPACKET) &&
            sk->sk_state == TCP_CLOSE)
while it can only be called as ->poll of unix_stream_ops, which means
that sk->sk_type can't be anything other that SOCK_STREAM in there.

static void scan_children(struct sock *x, void (*func)(struct unix_sock *),
                          struct sk_buff_head *hitlist)
{
        if (x->sk_state != TCP_LISTEN) {
                scan_inflight(x, func, hitlist);
        } else {
...
has no exclusion or barriers to deal with the store of TCP_LISTEN into
->sk_state inside unix_listen().  That one's potentially nasty - we won't
find SCM_RIGHTS already queued to embrios in x's queue until we notice
that x->sk_state == TCP_LISTEN, which can happen between two calls of
scan_children() in the same unix_gc() run.  The race is narrow, but not
impossible, AFAICS.  Reasonably easy to fix - lift locking the queue
out of scan_inflight(), grab the queue lock before checking if it's
a listener and have unix_listen() either grab the queue lock around the
assignment to ->sk_state, or pump it up and down before dropping
unix_state_lock() (at which point connect() might be able to find it, etc.)

Al, still digging through net/unix...

Re: [PATCH][unix] missing barriers in some of unix_sock ->addr and ->path accesses

[Sasha Levin]

On Mon, Feb 18, 2019 at 09:32:46PM +0000, Al Viro wrote:
>On Mon, Feb 18, 2019 at 09:14:33PM +0000, Sasha Levin wrote:
>> Hi,
>>
>> [This is an automated email]
>>
>> This commit has been processed because it contains a -stable tag.
>
>Ugh...  Should've removed Cc; stable from netdev posting; my apologies.
>
>> How should we proceed with this patch?
>
>Wait for it to get into davem's tree, for starters?

It won't go in stable until it's upstream, the only reason for this
early mail is that it's easier to get responses from people while the
patch is still fresh in their brains (as you might have noticed ;) )
rather than in a few weeks.

--
Thanks,
Sasha