From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1754142AbdCBQup (ORCPT <rfc822;w@1wt.eu>);
        Thu, 2 Mar 2017 11:50:45 -0500
Received: from mx1.redhat.com ([209.132.183.28]:43476 "EHLO mx1.redhat.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1753004AbdCBQui (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Thu, 2 Mar 2017 11:50:38 -0500
Organization: Red Hat UK Ltd. Registered Address: Red Hat UK Ltd, Amberley
        Place, 107-111 Peascod Street, Windsor, Berkshire, SI4 1TE, United
        Kingdom.
        Registered in England and Wales under Company Registration No. 3798903
From: David Howells <dhowells@redhat.com>
In-Reply-To: <383d28fd-9a64-77ac-b8c0-f23408afc0ce@canonical.com>
References: <383d28fd-9a64-77ac-b8c0-f23408afc0ce@canonical.com>
To: Colin Ian King <colin.king@canonical.com>
Cc: dhowells@redhat.com, "David S. Miller" <davem@davemloft.net>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: rxrpc: Fix deadlock between call creation and sendmsg/recvmsg
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <7781.1488471227.1@warthog.procyon.org.uk>
Date: Thu, 02 Mar 2017 16:13:47 +0000
Message-ID: <7782.1488471227@warthog.procyon.org.uk>
X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Thu, 02 Mar 2017 16:13:50 +0000 (UTC)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Colin Ian King <colin.king@canonical.com> wrote:

> I think the following part of the patch is problematic:
> 
>         call = rxrpc_find_call_by_user_ID(rx, user_call_ID);
>         if (!call) {
> +               ret = -EBADSLT;
>                 if (cmd != RXRPC_CMD_SEND_DATA)
> -                       return -EBADSLT;
> +                       goto error_release_sock;
> +               ret = -EBUSY;
> 
> At this point call is null, so the following code is performing a null
> pointer dereference on call when accessing call->state.
> 
> Detected by CoverityScan CID#1414316 ("Dereference after null check")
> 
> +               if (call->state == RXRPC_CALL_UNINITIALISED ||
> +                   call->state == RXRPC_CALL_CLIENT_AWAIT_CONN ||
> +                   call->state == RXRPC_CALL_SERVER_PREALLOC ||
> +                   call->state == RXRPC_CALL_SERVER_SECURING ||
> +                   call->state == RXRPC_CALL_SERVER_ACCEPTING)
> +                       goto error_release_sock;
>                 call = rxrpc_new_client_call_for_sendmsg(rx, msg,
> user_call_ID,

Good catch, thanks,  I put the state check in the wrong half of the
if-statement.

David