Message ID | 20231013225619.987912-1-anjali.k.kulkarni@oracle.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:612c:2908:b0:403:3b70:6f57 with SMTP id ib8csp2196205vqb; Fri, 13 Oct 2023 15:56:51 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFOqj9/NFZqruCLysi7IPeho9kD0V3iDcwURV/v24wrWJEiexOiuXGuD5AB03XEeTviNd8V X-Received: by 2002:a05:6a00:1d94:b0:690:d314:38d with SMTP id z20-20020a056a001d9400b00690d314038dmr31587190pfw.1.1697237811540; Fri, 13 Oct 2023 15:56:51 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1697237811; cv=none; d=google.com; s=arc-20160816; b=fnOy3e4JCD8d23nR9R+Zj+fKrUjYhzxHhzjQc1A8Ufpso5DoH3e9pOfuWwpJPmDbgx R88Zz+11I5khMmrx2/jl6scP9APVzw3YZdeTM9uBU6K/O7EZN6wigSuNCjjKHspxWe/U ohOk/Id0MJIGCjhiAWPlZ12sdGP3zl1I18Xdp2kQfPZ7Zin4WAKnnyAuxHs1Z93nWtHA fqk0qptsjwIZX5BZdMATb99mM09GO8PhoPd21mBppkR4EPwK4FuyT83OIkHAPbKBoypR 7yQ1B76D60+om60jB7ROgfV8gt824EdLiuhG1JLUNWDC9OajX9NxycdhoIIkFsrPI+80 KL9A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=tIv8PKWqTNvf7LEb9nHTdGBuAH32pLXCPsbisyTQ9h8=; fh=ITY154T5DdACbZ7QC3DriEUZnyFjuYVbuY+ukterbLk=; b=gFXMfw3FJe3470MYx9FHPmhPI6WdtiUiaZ2U/lBffrAm2hLe/0DILPVrYKUUvcpWpP /BzS555UmHm5Hh++Qc7qXm1hr0hIECXCiHJhKzhFLUyD7aNB77pbRrXHa1TAwC+gzbZf rFs62TtJA9VfGswGtvZ9ztTe2zfnsNHpYO2R8JyMXj/gz/vM/M+rUV7zdQ8qAPaFZ0S+ vhRwtRRT+1CQkaEkmSKNxhw6YVD5yOff8Nw9DtvGr4akFxue4A+KSG7pqiK5YGdM3sxc nLAYAL7eHw4rREwnfxhap/CF/aCEBz3cWf3qHq7T2+SKBvGUXCJ9qU2pq4AbGnLEf7H+ 4W4Q== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@oracle.com header.s=corp-2023-03-30 header.b=FGC7gDXD; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=oracle.com Received: from morse.vger.email (morse.vger.email. [23.128.96.31]) by mx.google.com with ESMTPS id c128-20020a633586000000b005ae03de31cfsi1441967pga.715.2023.10.13.15.56.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 13 Oct 2023 15:56:51 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31; Authentication-Results: mx.google.com; dkim=pass header.i=@oracle.com header.s=corp-2023-03-30 header.b=FGC7gDXD; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=oracle.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by morse.vger.email (Postfix) with ESMTP id 8D6E480AFBA8; Fri, 13 Oct 2023 15:56:49 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at morse.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232346AbjJMW4a (ORCPT <rfc822;rua109.linux@gmail.com> + 19 others); Fri, 13 Oct 2023 18:56:30 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36564 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229830AbjJMW42 (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 13 Oct 2023 18:56:28 -0400 Received: from mx0a-00069f02.pphosted.com (mx0a-00069f02.pphosted.com [205.220.165.32]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D538AB7; Fri, 13 Oct 2023 15:56:26 -0700 (PDT) Received: from pps.filterd (m0246617.ppops.net [127.0.0.1]) by mx0b-00069f02.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 39DLJNgG017941; Fri, 13 Oct 2023 22:56:23 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=from : to : cc : subject : date : message-id : mime-version : content-transfer-encoding; s=corp-2023-03-30; bh=tIv8PKWqTNvf7LEb9nHTdGBuAH32pLXCPsbisyTQ9h8=; b=FGC7gDXDIKQOgVqzozCMJCCPtZCS+Aq+iBD/z/VAiIDqyOOI9t+QVu6yI95XR3Zx8ZMy YovccVIDVSQyY/UVNEQ9nbwQkz0PDzp/z/j+0108StqdtwQzNSSRQ0NAi1V64f4bDn0e Y0ZH8f8lQrGNP6ZsXKdWZji5UfLyf84NLc7sC4TShdQ/MMYPheq9D9TsPm7WRdnuPLpA N/vOeAHz6L7r5Om09xUlvVIV3xoPl1zLpeVO7Awywg17AJDtKGZXv8NOG11+jRzBrbV/ kvMU8w7ylZW5+UQIjfPcXDLZ9P4BXnoPRwAFthXhSsUDbLWQCM1DHWn1dXv1gIyWPQ7P DA== Received: from phxpaimrmta01.imrmtpd1.prodappphxaev1.oraclevcn.com (phxpaimrmta01.appoci.oracle.com [138.1.114.2]) by mx0b-00069f02.pphosted.com (PPS) with ESMTPS id 3tjyvux1mj-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 13 Oct 2023 22:56:22 +0000 Received: from pps.filterd (phxpaimrmta01.imrmtpd1.prodappphxaev1.oraclevcn.com [127.0.0.1]) by phxpaimrmta01.imrmtpd1.prodappphxaev1.oraclevcn.com (8.17.1.19/8.17.1.19) with ESMTP id 39DL8isA006017; Fri, 13 Oct 2023 22:56:22 GMT Received: from pps.reinject (localhost [127.0.0.1]) by phxpaimrmta01.imrmtpd1.prodappphxaev1.oraclevcn.com (PPS) with ESMTPS id 3tptct2ukk-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 13 Oct 2023 22:56:22 +0000 Received: from phxpaimrmta01.imrmtpd1.prodappphxaev1.oraclevcn.com (phxpaimrmta01.imrmtpd1.prodappphxaev1.oraclevcn.com [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 39DMuL2a028304; Fri, 13 Oct 2023 22:56:21 GMT Received: from ca-dev112.us.oracle.com (ca-dev112.us.oracle.com [10.129.136.47]) by phxpaimrmta01.imrmtpd1.prodappphxaev1.oraclevcn.com (PPS) with ESMTP id 3tptct2uk4-1; Fri, 13 Oct 2023 22:56:21 +0000 From: Anjali Kulkarni <anjali.k.kulkarni@oracle.com> To: linux-kernel@vger.kernel.org Cc: davem@davemloft.net, Liam.Howlett@oracle.com, netdev@vger.kernel.org, oliver.sang@intel.com, kuba@kernel.org, horms@kernel.org, anjali.k.kulkarni@oracle.com Subject: [PATCH v1] Fix NULL pointer dereference in cn_filter() Date: Fri, 13 Oct 2023 15:56:19 -0700 Message-ID: <20231013225619.987912-1-anjali.k.kulkarni@oracle.com> X-Mailer: git-send-email 2.42.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.272,Aquarius:18.0.980,Hydra:6.0.619,FMLib:17.11.176.26 definitions=2023-10-13_12,2023-10-12_01,2023-05-22_02 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 suspectscore=0 malwarescore=0 adultscore=0 mlxscore=0 bulkscore=0 phishscore=0 spamscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2309180000 definitions=main-2310130199 X-Proofpoint-ORIG-GUID: hT7UL-RIcTeRO0UK7AxwyczkJvQ660aN X-Proofpoint-GUID: hT7UL-RIcTeRO0UK7AxwyczkJvQ660aN X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on morse.vger.email Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (morse.vger.email [0.0.0.0]); Fri, 13 Oct 2023 15:56:49 -0700 (PDT) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1779682835516326011 X-GMAIL-MSGID: 1779682835516326011 |
Series |
[v1] Fix NULL pointer dereference in cn_filter()
|
|
Commit Message
Anjali Kulkarni
Oct. 13, 2023, 10:56 p.m. UTC
Check that sk_user_data is not NULL, else return from cn_filter().
Fixes: 2aa1f7a1f47c ("connector/cn_proc: Add filtering to fix some bugs")
Reported-by: kernel test robot <oliver.sang@intel.com>
Closes: https://lore.kernel.org/oe-lkp/202309201456.84c19e27-oliver.sang@intel.com/
Signed-off-by: Anjali Kulkarni <anjali.k.kulkarni@oracle.com>
---
drivers/connector/cn_proc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Comments
On Fri, Oct 13, 2023 at 03:56:19PM -0700, Anjali Kulkarni wrote: > Check that sk_user_data is not NULL, else return from cn_filter(). Thanks, I agree that this change seems likely to address the problem at the link below. And I also think cn_filter() is a good place to fix this [1]. But I am wondering if you could add some commentary to the patch description, describing under what circumstances this problem can occur. [1] https://lore.kernel.org/all/20231013120105.GH29570@kernel.org/ > Fixes: 2aa1f7a1f47c ("connector/cn_proc: Add filtering to fix some bugs") > Reported-by: kernel test robot <oliver.sang@intel.com> > Closes: https://lore.kernel.org/oe-lkp/202309201456.84c19e27-oliver.sang@intel.com/ > Signed-off-by: Anjali Kulkarni <anjali.k.kulkarni@oracle.com> > --- > drivers/connector/cn_proc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c > index 05d562e9c8b1..a8e55569e4f5 100644 > --- a/drivers/connector/cn_proc.c > +++ b/drivers/connector/cn_proc.c > @@ -54,7 +54,7 @@ static int cn_filter(struct sock *dsk, struct sk_buff *skb, void *data) > enum proc_cn_mcast_op mc_op; > uintptr_t val; > > - if (!dsk || !data) > + if (!dsk || !data || !dsk->sk_user_data) > return 0; > > ptr = (__u32 *)data; > -- > 2.42.0 >
> On Oct 17, 2023, at 1:02 AM, Simon Horman <horms@kernel.org> wrote: > > On Fri, Oct 13, 2023 at 03:56:19PM -0700, Anjali Kulkarni wrote: >> Check that sk_user_data is not NULL, else return from cn_filter(). > > Thanks, > > I agree that this change seems likely to address the problem at the link > below. And I also think cn_filter() is a good place to fix this [1]. > But I am wondering if you could add some commentary to the patch > description, describing under what circumstances this problem can occur. Thanks for reviewing! Yes, it seems this patch has fixed the issue (as tested by Oliver Sang). I will add some description for when this problem can occur in the next patch revision. > > [1] https://urldefense.com/v3/__https://lore.kernel.org/all/20231013120105.GH29570@kernel.org/__;!!ACWV5N9M2RV99hQ!NNsMHaho3lyW2NyoT8Sju1BL78S5pNu5AhtZC76cc1Xb1_psXwfP0lmVtVmTxGRrsQkzClWNS8HriosTMols$ > >> Fixes: 2aa1f7a1f47c ("connector/cn_proc: Add filtering to fix some bugs") >> Reported-by: kernel test robot <oliver.sang@intel.com> >> Closes: https://urldefense.com/v3/__https://lore.kernel.org/oe-lkp/202309201456.84c19e27-oliver.sang@intel.com/__;!!ACWV5N9M2RV99hQ!NNsMHaho3lyW2NyoT8Sju1BL78S5pNu5AhtZC76cc1Xb1_psXwfP0lmVtVmTxGRrsQkzClWNS8HrirVfNms7$ >> Signed-off-by: Anjali Kulkarni <anjali.k.kulkarni@oracle.com> >> --- >> drivers/connector/cn_proc.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c >> index 05d562e9c8b1..a8e55569e4f5 100644 >> --- a/drivers/connector/cn_proc.c >> +++ b/drivers/connector/cn_proc.c >> @@ -54,7 +54,7 @@ static int cn_filter(struct sock *dsk, struct sk_buff *skb, void *data) >> enum proc_cn_mcast_op mc_op; >> uintptr_t val; >> >> - if (!dsk || !data) >> + if (!dsk || !data || !dsk->sk_user_data) >> return 0; >> >> ptr = (__u32 *)data; >> -- >> 2.42.0 >>
> On Oct 17, 2023, at 1:02 AM, Simon Horman <horms@kernel.org> wrote: > > On Fri, Oct 13, 2023 at 03:56:19PM -0700, Anjali Kulkarni wrote: >> Check that sk_user_data is not NULL, else return from cn_filter(). > > Thanks, > > I agree that this change seems likely to address the problem at the link > below. And I also think cn_filter() is a good place to fix this [1]. > But I am wondering if you could add some commentary to the patch > description, describing under what circumstances this problem can occur. Hi, We always allocate sk_user_data in cn_proc_mcast_ctl(), which should ideally happen before any fork event is triggered, which ends up calling cn_filter(), where we need to use sk_user_data. So in the normal flow of events, sk_user_data should not be NULL in cn_filter(). I also looked at whether there were any race conditions which could cause this issue and found one possibility - if a fork event was triggered between the time that the bind() call was made by the application (which ends up adding the socket to the multicast list for CONNECTOR maintained by netlink layer), and the call to cn_proc_mcast_ctl(), then the netlink layer will try to send the event to the sockets in the multicast list via cn_filter() and find that sk_user_data is NULL. However, when I tried to reproduce this case, I could not do it by adding a wait between bind() & cn_proc_mcast_ctl(). On looking at the code, I did see that in proc_fork_connector(), we do check for proc_event_num_listeners being greater than 0 before we invoke netlink layer's send message, which calls cn_filter(). And proc_event_num_listeners is only incremented after sk_user_data is allocated. This is why we cannot reproduce it this way. Perhaps there is some other way I am not aware of, but I could not think of anything else. I will just resend the patch. Thanks Anjali > > [1] https://urldefense.com/v3/__https://lore.kernel.org/all/20231013120105.GH29570@kernel.org/__;!!ACWV5N9M2RV99hQ!NNsMHaho3lyW2NyoT8Sju1BL78S5pNu5AhtZC76cc1Xb1_psXwfP0lmVtVmTxGRrsQkzClWNS8HriosTMols$ > >> Fixes: 2aa1f7a1f47c ("connector/cn_proc: Add filtering to fix some bugs") >> Reported-by: kernel test robot <oliver.sang@intel.com> >> Closes: https://urldefense.com/v3/__https://lore.kernel.org/oe-lkp/202309201456.84c19e27-oliver.sang@intel.com/__;!!ACWV5N9M2RV99hQ!NNsMHaho3lyW2NyoT8Sju1BL78S5pNu5AhtZC76cc1Xb1_psXwfP0lmVtVmTxGRrsQkzClWNS8HrirVfNms7$ >> Signed-off-by: Anjali Kulkarni <anjali.k.kulkarni@oracle.com> >> --- >> drivers/connector/cn_proc.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c >> index 05d562e9c8b1..a8e55569e4f5 100644 >> --- a/drivers/connector/cn_proc.c >> +++ b/drivers/connector/cn_proc.c >> @@ -54,7 +54,7 @@ static int cn_filter(struct sock *dsk, struct sk_buff *skb, void *data) >> enum proc_cn_mcast_op mc_op; >> uintptr_t val; >> >> - if (!dsk || !data) >> + if (!dsk || !data || !dsk->sk_user_data) >> return 0; >> >> ptr = (__u32 *)data; >> -- >> 2.42.0 >>
diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c index 05d562e9c8b1..a8e55569e4f5 100644 --- a/drivers/connector/cn_proc.c +++ b/drivers/connector/cn_proc.c @@ -54,7 +54,7 @@ static int cn_filter(struct sock *dsk, struct sk_buff *skb, void *data) enum proc_cn_mcast_op mc_op; uintptr_t val; - if (!dsk || !data) + if (!dsk || !data || !dsk->sk_user_data) return 0; ptr = (__u32 *)data;