Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 28 Sep 2021 15:23:48 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     Pavel Skripkin <paskripkin@gmail.com>
Cc:     Yanfei Xu <yanfei.xu@windriver.com>,
        Bartosz Golaszewski <bgolaszewski@baylibre.com>,
        andrew@lunn.ch, hkallweit1@gmail.com, linux@armlinux.org.uk,
        davem@davemloft.net, kuba@kernel.org, p.zabel@pengutronix.de,
        syzbot+398e7dc692ddbbb4cfec@syzkaller.appspotmail.com,
        linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
        Dongliang Mu <mudongliangabcd@gmail.com>
Subject: Re: [PATCH] net: mdiobus: Fix memory leak in __mdiobus_register
Message-ID: <20210928122348.GO2083@kadam>
References: <20210928085549.GA6559@kili>
 <20210928092657.GI2048@kadam>
 <6f90fa0f-6d3b-0ca7-e894-eb971b3b69fa@gmail.com>
 <20210928103908.GJ2048@kadam>
 <63b18426-c39e-d898-08fb-8bfd05b7be9e@gmail.com>
 <20210928105943.GL2083@kadam>
 <283d01f0-d5eb-914e-1bd2-baae0420073c@gmail.com>
 <f587da4b-09dd-4c32-4ee4-5ec8b9ad792f@gmail.com>
 <20210928113055.GN2083@kadam>
 <c86fecd6-6412-fec8-1dce-81e99c059e38@gmail.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <c86fecd6-6412-fec8-1dce-81e99c059e38@gmail.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?MJVVT9KXzwXy7MTt2kVLMmNXpF7DpR8gxLZohSwPDFkUvscb/L+ofReKpWQI?=
 =?us-ascii?Q?K9YNg1M9ie+yDl2fvwsZzlKYtRGV4VRNHfXqrcQLdjv+SmpEMiWeWYR+A2Sk?=
 =?us-ascii?Q?GkYrJj/PXAZ76hle7IV3jWFh8pDFaeiwqkVLyZcbr7L13eFI6uCEdNeFDFuv?=
 =?us-ascii?Q?qzycSsHmTiXhT9YBbvN7emGZiI/1MGGaMGleyX8m+Qn4fG5WLITp/up2UuYV?=
 =?us-ascii?Q?mpUmXVVvI1XBcEhpJ5n4IcZGoR60OsQdGGjXurwPSLf47N8XV03uhzBMMWpX?=
 =?us-ascii?Q?gpkBwO6oy1DHTKJ1X4ngL/zTSyc4HaqSPQMoNsZymzAQi1VCIk9g8nrEy38M?=
 =?us-ascii?Q?TfWAYe59cIYmj8QTV+YCZo5AGpyiuUP4yh0Dr0VFxJYj7JbcF6UPvKyx06eD?=
 =?us-ascii?Q?pzacerYh+4skeFrEeVvMhb/KJTBc/IQ8oD9q2pWs97xUJMv+HN16jDm5nHgJ?=
 =?us-ascii?Q?lzB7SH2Nybw7UYOzfOccx0pXZ6RKYgwiBRNRKTP8IsmNaDrSTIIsyx062+lG?=
 =?us-ascii?Q?W8rPCYOcfA/6F/rRTgeF5CeyFE+HA7l9UdNYOwvdi66pOTcxQtt40EsZnzvJ?=
 =?us-ascii?Q?Usy7vvKH3uLzGqB3sYba2avIDRI3g0mBm1OFtp4RWIxI+20xkhQSFK4PEZwp?=
 =?us-ascii?Q?nLeB/ftdKTTGecDpt5wm9pRYeeBYgI3Aw/Xtu3E1m8StL/0mXqpYzNrTg060?=
 =?us-ascii?Q?BRDgvDU6pDmhCQ/NutoEyhrS0bLpZeN8lntzw23xb5Cyvaw9FgqvIu/bRrd/?=
 =?us-ascii?Q?pBi04rNmjBgqgNVLOhGsNKVpnJb0V7pONRbgdx7bc71syYnFgAewZhc5ELEg?=
 =?us-ascii?Q?15R/eIOgpfD+ybClXNKLq6JwZAPUneM+JSkrNrBa4Lo31CfJoFI/KfFOc+dq?=
 =?us-ascii?Q?pIVk0ZiO9+EyJKzb4ZOt7ON7xYUVQKPnnRbTf4GH/mtWYrgmMx91aiaiA6EI?=
 =?us-ascii?Q?mryYxrTX6eBP+P+Z9GvQHp59eAZs7ysuDefsaEn1V7pHx/vdogDSTbL71dwy?=
 =?us-ascii?Q?x0EDYMRif8TmfO6bqdbdNKJ8M7+YqSAse2/s+b+l8/yLL1CuvzFjeuHlWeIc?=
 =?us-ascii?Q?eFzNL6gXFVHMqpqfA9TKwb9FQtdkrOGuMiQ3oUqVGxS/ZDLnkGTgKZJ9O1Oc?=
 =?us-ascii?Q?wCEAdjQHhaW5WqR5I+mnS1WKGz7shDpnf3dESCInPTy+iWWE0qeEhR35N0Vq?=
 =?us-ascii?Q?cGPtqdLDhSfYDR4WkR4/kVkR2UTBhOgAtBzbkBsBFcKm5hlzayvaQDhosk3a?=
 =?us-ascii?Q?qrUHpnspKuG2LTad+D9ZtO+75/TsgPAZvXVqa5c8nmfFBcQ5o3BNLyYB7aJF?=
 =?us-ascii?Q?NhgKASyXp70BHo+KLW+HNEPL?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 5b4189d7-838c-483a-14bf-08d9827adea0
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1001MB2365.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Sep 2021 12:24:08.4728
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Dg6KrSboTul7i6FmUMMgzN4ybAuwPtFbdO978LFnafP3ty+hVL1cx9ny8lTkEVrqhxqRBz/RzF29m86tQrL6m4wzxhJd1+y4vPh71j8cT0I=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO6PR10MB5460
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10120 signatures=668682
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 suspectscore=0
 malwarescore=0 spamscore=0 adultscore=0 bulkscore=0 mlxscore=0
 mlxlogscore=999 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2109230001 definitions=main-2109280070
X-Proofpoint-GUID: g9ax_bI5C2mqe_T1p88K9wa7UogjhcHD
X-Proofpoint-ORIG-GUID: g9ax_bI5C2mqe_T1p88K9wa7UogjhcHD
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Tue, Sep 28, 2021 at 02:45:39PM +0300, Pavel Skripkin wrote:
> On 9/28/21 14:30, Dan Carpenter wrote:
> > On Tue, Sep 28, 2021 at 02:09:06PM +0300, Pavel Skripkin wrote:
> >
> > Huh...  You're right that the log should say "failed to register".  But
> > I don't think that's the correct syzbot link for your patch either
> > because I don't think anyone calls mdiobus_free() if
> > __devm_mdiobus_register() fails.  I have looked at these callers.  It
> > would be a bug as well.
> > 
> 
> mdiobus_free() is called in case of ->probe() failure, because devres clean
> up function for bus is devm_mdiobus_free(). It simply calls mdiobus_free().
> 
> 
> So, i imagine following calltrace:
> 
> ax88772_bind
>   ax88772_init_mdio
>     devm_mdiobus_alloc() <- bus registered as devres
>     devm_mdiobus_register() <- fail (->probe failure)
> 
> ...
> 
> devres_release_all
>   mdiobus_free()

Argh... Crap.  You're right.  There is just one bug.  No need to
change __devm_mdiobus_register() and trying to do that would lead to a
UAF.

Your patch is the correct fix but with the modifications we discussed.

regards,
dan carpenter