Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) client-ip=2620:137:e000::3:5;
From:   Suman Ghosh <sumang@marvell.com>
To:     Siddh Raman Pant <code@siddh.me>,
        Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>
CC:     "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Samuel Ortiz <sameo@linux.intel.com>,
        "syzbot+bbe84a4010eeea00982d@syzkaller.appspotmail.com" 
        <syzbot+bbe84a4010eeea00982d@syzkaller.appspotmail.com>
Subject: RE: [EXT] [PATCH net-next v2 1/2] nfc: llcp_core: Hold a ref to
 llcp_local->dev when holding a ref to llcp_local
Thread-Topic: [EXT] [PATCH net-next v2 1/2] nfc: llcp_core: Hold a ref to
 llcp_local->dev when holding a ref to llcp_local
Thread-Index: AQHaJTYgngNuT7kSE0OBtZPJVIp8E7CXuLWQ
Date:   Sun, 3 Dec 2023 16:59:39 +0000
Message-ID: <SJ0PR18MB5216AB7B26C1F34A8748F2DADB87A@SJ0PR18MB5216.namprd18.prod.outlook.com>
References: <cover.1701530776.git.code@siddh.me>
 <476cccdcb57645784889fc82f0c7c10ff4c8b8c0.1701530776.git.code@siddh.me>
In-Reply-To: <476cccdcb57645784889fc82f0c7c10ff4c8b8c0.1701530776.git.code@siddh.me>
Accept-Language: en-IN, en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?nRL0pOs7EKDnUlTu1L3yc8vCyCP+WbNTgITMG/LOHwotNfs5veI0rQV7bUpk?=
 =?us-ascii?Q?AX6gmWftqZbmmgNaTtMbH22e6RGYJ4mZ1vzu2QTTAgV44WfGLtdA6zOjoEeD?=
 =?us-ascii?Q?/+fNmATeeUumn/bygNy/LOZ8U12LPWz3/clt+uTt7aQ/uwLVqP6zUo7QR0PK?=
 =?us-ascii?Q?ef9iVV53/QcdjaROnQ3n2jTaqzCF1Is1Y4jTN2osILSvA+WcM8lb93TxCezN?=
 =?us-ascii?Q?/FxYY6A/IUfiJy/eu2xB3dMuWQJca3xSJSWiudXYyOFdd8PGkurblgOrQcXA?=
 =?us-ascii?Q?mcwo0V3mO03sdNaK7ZhzGDB1+PCsmi3yJP1KVLUNhylY40lAH1nwUTn1Fv6R?=
 =?us-ascii?Q?8zf6qIubbTkKn8rQ8EbL6Vbv4MFqZhHuAl6sBNbDi1XhmZPDbmYMc7xWw6YG?=
 =?us-ascii?Q?8sopltOecTHEiJpocsDBDEKILpSscy7w/vd0x9r/rqxgiPkAbezJ9IM6h2zK?=
 =?us-ascii?Q?Dtr5drarSQdTI/UMBqG44CEqinLUhuIJxI3gpgw8Wj2SvZfrL+BOimrj0a1K?=
 =?us-ascii?Q?bUr3NFsIvJ9w3JcKFQPLujdfNtgFx5ZdrMit2nOoB+kANfFJYgDL3OnZq7m2?=
 =?us-ascii?Q?9ucar7D0ICGJZp2jESkP1+F/QmsL2Td7e+ApzkWyXPaRLcBt2iEtn7FKPJz3?=
 =?us-ascii?Q?417QiQaZOPEfYju2spg4WHX+USjPa7L/w7uSPrICIOuqe+evfvaOwFKQ5nAM?=
 =?us-ascii?Q?KhcenPK1/vPM4FwnS+zhax/6X8nMB0eCaFevYnZlKtko5tXiWSQZKvc3aPQh?=
 =?us-ascii?Q?X/oIfrBAuWCZLBIPypbLJsPYuC1BdCD2MmRav1rhGgyEdSQ4izXZNw3+JYrp?=
 =?us-ascii?Q?xU1CVwZfs2/ITYUmPn01UARjtE5bZIwHD+iiW1ydejPKsk0HojyAYWZvsrHf?=
 =?us-ascii?Q?9vVYNI4ijA88/cLBxtcHEicJDzDmZ2LFTXggpox67aPjcFAoKQS52XO2XicU?=
 =?us-ascii?Q?M0xnv7ETVmYcepN3O/VlUIKADoOvJkax+DLKSOu4wY1uSOlvKHw/9GnMjkut?=
 =?us-ascii?Q?7uNe9dZS3IxWmkYmDTnL/feq/EsD4V2NSDn90pkRRZwXY8EPOz4GQaJi/Yxx?=
 =?us-ascii?Q?/Xl2Kxlf4QH+sfJpzhSLReqvwg95F1a4ZmNHnWMI8dTTzlneEbQ6XWFNutvE?=
 =?us-ascii?Q?0SPhJ652uhIjlIzer1xWeT11pit8Jd/p9euxn1/9dlOJvNsDuParbNZb/9el?=
 =?us-ascii?Q?c2VwSjmEIrGm9OhP/jDNoDEhe8jC7iWcO4B3jMdB8QBRS1yODQYcqXaUPJvy?=
 =?us-ascii?Q?VG8/jdvr2rd89KFDz8Su4N7jm0FEGuek7yUJZcAwlyPtrir5rkEfoPCj/WLJ?=
 =?us-ascii?Q?OkPKT8aNB9HdIcJS33U9oDxfFHFj7Bbz9mi/kIzwCLe+D15nriQF3QDN+NxJ?=
 =?us-ascii?Q?0KzTjvdwXARdA1S1QnUYyHiDEToONY+WPdoPhV4Jlg8RWFKslAAjPCGi/Itl?=
 =?us-ascii?Q?F5gdGVRJPj/a7GiTe/4iB324a3uRvY6V7sfPVpzEtqskcAtU1I/s/C+9ObZ7?=
 =?us-ascii?Q?7MBcwvW8H48N+spLaYPooP2Cs5+mw4qnocvUSEnv5XmWsN+S3Oao9LXpWMj9?=
 =?us-ascii?Q?fACbhgIV+HpoDUJMkvLGSU0mrWgZZt3kRRcNzqYP?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: marvell.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: SJ0PR18MB5216.namprd18.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: fb4bcf09-9688-458d-d89c-08dbf4213ccd
X-MS-Exchange-CrossTenant-originalarrivaltime: 03 Dec 2023 16:59:39.3976
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 70e1fb47-1155-421d-87fc-2e58f638b6e0
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: X9rVFW06ulbwJrfNQbsogcO8vwlVLT8G+gS1DmXA1n1d/jll5Qbafcc1PI1qGkUtVGBgOpVx/QRg05rAgJYUQQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR18MB4317
X-Proofpoint-ORIG-GUID: z0adMqWCmVpJisU2v82Rn-Y9eOn42VJu
X-Proofpoint-GUID: z0adMqWCmVpJisU2v82Rn-Y9eOn42VJu
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.997,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2023-12-03_15,2023-11-30_01,2023-05-22_02
X-Spam-Status: No, score=-0.8 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,
	SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no
	version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on groat.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 (groat.vger.email [0.0.0.0]); Sun, 03 Dec 2023 09:00:17 -0800 (PST)

Hi Siddh,

>@@ -180,6 +183,7 @@ int nfc_llcp_local_put(struct nfc_llcp_local *local)
> 	if (local =3D=3D NULL)
> 		return 0;
>
>+	nfc_put_device(local->dev);
[Suman] One question here, if we consider the path, nfc_llcp_mac_is_down() =
-> nfc_llcp_socket_release() -> nfc_llcp_local_put(), then inside nfc_llcp_=
socket_release()
we are already doing nfc_put_device() if "sk->sk_state =3D=3D LLCP_CONNECTE=
D", with this change we are doing it again. I guess you need to add some ch=
eck to avoid that. Let me know if I am missing something.

> 	return kref_put(&local->ref, local_release);  }
>
>@@ -959,8 +963,17 @@ static void nfc_llcp_recv_connect(struct
>nfc_llcp_local *local,
> 	}
>
> 	new_sock =3D nfc_llcp_sock(new_sk);
>-	new_sock->dev =3D local->dev;
>+
> 	new_sock->local =3D nfc_llcp_local_get(local);
>+	if (!new_sock->local) {
>+		reason =3D LLCP_DM_REJ;
>+		release_sock(&sock->sk);
>+		sock_put(&sock->sk);
>+		sock_put(&new_sock->sk);
[Suman] don't we need to free new_sock? nfc_llcp_sock_free()?
>+		goto fail;
>+	}
>+
>+	new_sock->dev =3D local->dev;
> 	new_sock->rw =3D sock->rw;
> 	new_sock->miux =3D sock->miux;
> 	new_sock->nfc_protocol =3D sock->nfc_protocol; @@ -1597,7 +1610,11 @@
>int nfc_llcp_register_device(struct nfc_dev *ndev)
> 	if (local =3D=3D NULL)
> 		return -ENOMEM;
>
>-	local->dev =3D ndev;
>+	/* Hold a reference to the device. */
>+	local->dev =3D nfc_get_device(ndev->idx);
>+	if (!local->dev)
>+		return -ENODEV;
[Suman] Memory leak here. Need to call kfree(local).
>+
> 	INIT_LIST_HEAD(&local->list);
> 	kref_init(&local->ref);
> 	mutex_init(&local->sdp_lock);
>--
>2.42.0
>