On Thu, 16 Dec 2021, Lee Jones wrote:

On Thu, 16 Dec 2021, Xin Long wrote:

quoted

On Thu, Dec 16, 2021 at 11:39 AM Lee Jones [off-list ref] wrote:

quoted

On Thu, 16 Dec 2021, Xin Long wrote:

quoted

On Wed, Dec 15, 2021 at 8:48 PM Jakub Kicinski [off-list ref] wrote:

quoted

On Tue, 14 Dec 2021 21:57:32 +0000 Lee Jones wrote:

quoted

The cause of the resultant dump_stack() reported below is a
dereference of a freed pointer to 'struct sctp_endpoint' in
sctp_sock_dump().

This race condition occurs when a transport is cached into its
associated hash table followed by an endpoint/sock migration to a new
association in sctp_assoc_migrate() prior to their subsequent use in
sctp_diag_dump() which uses sctp_for_each_transport() to walk the hash
table calling into sctp_sock_dump() where the dereference occurs.

quoted

in sctp_sock_dump():
        struct sock *sk = ep->base.sk;
        ... <--[1]
        lock_sock(sk);

Do you mean in [1], the sk is peeled off and gets freed elsewhere?

'ep' and 'sk' are both switched out for new ones in sctp_sock_migrate().

quoted

if that's true, it's still late to do sock_hold(sk) in your this patch.

No, that's not right.

The schedule happens *inside* the lock_sock() call.

Sorry, I don't follow this.
We can't expect when the schedule happens, why do you think this
can never be scheduled before the lock_sock() call?

True, but I've had this running for hours and it hasn't reproduced.

Without this patch, I can reproduce this in around 2 seconds.

The C-repro for this is pretty intense!

If you want to be *sure* that a schedule will never happen, we can
take a reference directly with:

     ep = sctp_endpoint_hold(tsp->asoc->ep);
     sk = sock_hold(ep->base.sk);

Which was my original plan before I soak tested this submitted patch
for hours without any sign of reproducing the issue.

quoted

If the sock is peeled off or is being freed, we shouldn't dump this sock,
and it's better to skip it.

I guess we can do that too.

Are you suggesting sctp_sock_migrate() as the call site?

Also, when are you planning on testing the flag?

Won't that suffer with the same issue(s)?

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog