Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 3 Apr 2023 11:56:50 +0200
From:   Simon Horman <simon.horman@corigine.com>
To:     michenyuan <michenyuan@huawei.com>
Cc:     "isdn@linux-pingi.de" <isdn@linux-pingi.de>,
        "marcel@holtmann.org" <marcel@holtmann.org>,
        "johan.hedberg@gmail.com" <johan.hedberg@gmail.com>,
        "luiz.dentz@gmail.com" <luiz.dentz@gmail.com>,
        "davem@davemloft.net" <davem@davemloft.net>,
        "edumazet@google.com" <edumazet@google.com>,
        "kuba@kernel.org" <kuba@kernel.org>,
        "pabeni@redhat.com" <pabeni@redhat.com>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] cmtp: fix argument error
Message-ID: <ZCqi4laBvhCLrPhS@corigine.com>
References: <9b58282ff4ed4d2daad72539466c685d@huawei.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <9b58282ff4ed4d2daad72539466c685d@huawei.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?BRmPKm0Tuo4ZSqPuXmgQoPpj+A4mcYRR4Ef9LH98vJH/osFnvFINqHIgF24T?=
 =?us-ascii?Q?zcQu2EKg+PIhLms/eFa9FeNI8kpIsc1z/K7tWbnlYuxLvpzjE2hYkIhSy8N4?=
 =?us-ascii?Q?WdKNWH9pTNHPrggiQLqLDDGC2e317ZGK/0N9f95WJ5HxUPrDTfGRYPwHI4yI?=
 =?us-ascii?Q?fk+PmCcDKcOfAVUDxMi+glK3Nv3lmxewAqwhAvxXe/Y1ldzwpZpxwx3Lgg38?=
 =?us-ascii?Q?VK7vQa8Wzykegup/mHlbudK5nUUgkHWpUmEGKMIC4Bo1CfTW19qr99sC5Oor?=
 =?us-ascii?Q?/gSSITOnJvAvQ5cThu7HzlkOUGAUSdB6oIKCJQ5QKDKK3imGOLZ253ckZC5W?=
 =?us-ascii?Q?N2Ru4s703LpCsTxIBrRyZNDHEmGwd/Ei2bpyTJaJVtUpGRge87Py7n5IrvV/?=
 =?us-ascii?Q?fJuYltWNY99r/C3zGRMlRFB6g6LFOnhIrBzGjniz1LsSj/K4DpDEYPve+Lys?=
 =?us-ascii?Q?Zrcrq08FKEeKTR7jtjG5qPX3Kj66MEe//ddVY3sSJ6pP9EmKz0DuyvVxI58s?=
 =?us-ascii?Q?y8JSvxko4w2+YcryEHV4LSo2VsMqYyKnyWitUVlCkGSLVj1eBs5hFuAq2fmJ?=
 =?us-ascii?Q?HcohfaZr9u33LPFEt7Gl9ncGyCOyLmWYvECkSZbYFvr7zIRKnHAJsUKae0/v?=
 =?us-ascii?Q?TkJ+c51zgDorBaxhXnED7yH94GfcLEhpcebcxxLFMEUr/DVkXaX79Wa4Eh+m?=
 =?us-ascii?Q?iDjwM324MjJW0ACMU/2V4SWCz5p+9unFSnCbwgjBSSG40d1Ec2vZm3HFGFqW?=
 =?us-ascii?Q?pDMgfVwBvJUUG2tLhqjyHAMYTUN8O1LjHcNbVWb72Ubj+9XnfxFz8owGbr7I?=
 =?us-ascii?Q?NZ+OSahmAF308c43x13qLT/98GGo6NPV/FzLc+mgwVOqBpCtw2sbz3uCcoWP?=
 =?us-ascii?Q?pVogM/wPKh7KaTDT+ExlasaaDO7f16IWBLMrJqUWsIxU/g7EL1ErkwOMxdFA?=
 =?us-ascii?Q?yXEAMb8nV90/x4h4+bC7o6HhJi+7Qrt6mMeAEjC08WAWvCTtCOl01WB2sIx4?=
 =?us-ascii?Q?nh7a86nOjoE+hmKHfFsBDxgq4w6pPm/fE9eW61/5ieniXFpylrvtKbbyZpBE?=
 =?us-ascii?Q?YlwCsR7vlBRXVx4BH0D2WxP+aBVzETbheqmYJ242V68gxTD7ccHCHUIMLkHQ?=
 =?us-ascii?Q?s0XtdrEonRmKRrADyGTXxhaV9LF5zVO2l+hJIzF2Ps2Wt52EvAHQ+Upw4lU6?=
 =?us-ascii?Q?bq/ZiADbt+GqXH0lj0SG0JkbjFuT/wRtvAMpWdlrBLRCpNEU3e0SoCG66H20?=
 =?us-ascii?Q?Gk/Zgj/myd5sS+AAeM6cKgv6aUQvdwxrfYDiDKvntuz3rOqvcWAXCm+qIRZ2?=
 =?us-ascii?Q?JoyEODckt3JYL3e9sBV3DMgMEtsvakRDLcF1pWoV7srAehPnE2XswIMhi/iD?=
 =?us-ascii?Q?6xV6NdEmwyisTxA5JtDTo1MGlZSAL8mhyWVDU1TiHLBbp4oxRgy5zke27wDS?=
 =?us-ascii?Q?ItV1B4R8eHS7mx/6gOB7Jq2vf/bhTZjg8Wi86p81Mp7hT1Dn30zf1+btMz54?=
 =?us-ascii?Q?zj2bjuKaqq6ccl6pGcmTlWKzXMBBiOmYqSMcohV7w9/0kPBbBbZc5AsCDzV5?=
 =?us-ascii?Q?v2dT2o78MHpub1OCuAQPZlcNuuCsc7p4jDtNborR3QGMY3UTp8gjF7LnF3SR?=
 =?us-ascii?Q?LloJ94DFZOBv17pJzx3zj6EmVbOVe5adJVfDnL5RGS8FnWfx3IiKb7NwMG/4?=
 =?us-ascii?Q?7oHaTg=3D=3D?=
X-OriginatorOrg: corigine.com
X-MS-Exchange-CrossTenant-Network-Message-Id: bc2f320b-4c39-4548-a4af-08db3429c30e
X-MS-Exchange-CrossTenant-AuthSource: PH0PR13MB4842.namprd13.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Apr 2023 09:56:57.6076
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: fe128f2c-073b-4c20-818e-7246a585940c
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: y+N2vxnK0f2mzZ3YhNNV9bmYih6raQ80IceRivr+++KY6U4IGamd2iWjeUuD9tIpkNJ2kiBpCViYCFRP2j5mpHnW/C4zdQF0oa6ls4g5BCQ=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR13MB4603
X-Spam-Status: No, score=-0.0 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
        RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS
        autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-bluetooth.vger.kernel.org>
X-Mailing-List: linux-bluetooth@vger.kernel.org

On Mon, Apr 03, 2023 at 02:35:21AM +0000, michenyuan wrote:
> Thank you for your suggestion.
> 
> This bug may not cause serious security problem. Function 'bt_sock_unregister' takes its parameter as an index and nulls the corresponding element of 'bt_proto' which is an array of pointers. When 'bt_proto' dereferences each element, it would check whether the element is empty or not. Therefore, the problem of null pointer deference does not occur.
> 
> This bug is observed by manually code review.

Thanks, could I suggest that you post a v2 that looks a bit like this:

Subject: Re: [PATCH v2 net-next] bluetooth: unregister correct BTPROTO for CMTP

On error unregister BTPROTO_CMTP to match the registration earlier
in the same code-path. Without this change BTPROTO_HIDP is incorrectly
unregistered.

This bug does not appear to cause serious security problem.

The function 'bt_sock_unregister' takes its parameter as an index and NULLs
the corresponding element of 'bt_proto' which is an array of pointers. When
'bt_proto' dereferences each element, it would check whether the element is
empty or not. Therefore, the problem of null pointer deference does not
occur.

Found by inspection.

Fixes: 8c8de589cedd ("Bluetooth: Added /proc/net/cmtp via bt_procfs_init()")
Signed-off-by: ...

...

> > ---
> >  net/bluetooth/cmtp/sock.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Code change looks good.
> 
> > diff --git a/net/bluetooth/cmtp/sock.c b/net/bluetooth/cmtp/sock.c 
> > index 96d49d9fae96..cf4370055ce2 100644
> > --- a/net/bluetooth/cmtp/sock.c
> > +++ b/net/bluetooth/cmtp/sock.c
> > @@ -250,7 +250,7 @@ int cmtp_init_sockets(void)
> >  	err = bt_procfs_init(&init_net, "cmtp", &cmtp_sk_list, NULL);
> >  	if (err < 0) {
> >  		BT_ERR("Failed to create CMTP proc file");
> > -		bt_sock_unregister(BTPROTO_HIDP);
> > +		bt_sock_unregister(BTPROTO_CMTP);
> >  		goto error;
> >  	}
> >  
> > --
> > 2.25.1