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 14:30:55 +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: <20210928113055.GN2083@kadam>
References: <20210926045313.2267655-1-yanfei.xu@windriver.com>
 <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>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <f587da4b-09dd-4c32-4ee4-5ec8b9ad792f@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?ZSU6w4C/t5GoGfxcmOVMVMuOavkko6FyB3lGPLjHsG1hiYnTHgBZCBnBrq2C?=
 =?us-ascii?Q?dqOKzNy4WiLE7jKgEr3JLkqjtMjyhDG0T3gNBOh1lGd7Zeut3sVtVVONqzKg?=
 =?us-ascii?Q?h8I/ZH7yjiZ26r0Gs4tDW5coKeBCEtPTN7sKFtY76DMRq2DC9rYmXM6J7vIB?=
 =?us-ascii?Q?l47gBpaamL73H/fxEmqBPmkrHjoFP9xft8wc0lTjWqAlnKhsQSVoJA4e1J5o?=
 =?us-ascii?Q?je6wLN3a95mG9a9whEwEf2x7wipPaESZC3k0QrJ304/wnZMBZ3hhw1lfA2Z4?=
 =?us-ascii?Q?+x9eWwkbZfoZiZR1WbNoi+mKVV5D4k5F9XQZbot5h9RfxVSPnot23PrskxRn?=
 =?us-ascii?Q?b2MXkn+lpfo87ml2Nu2QYlX+YQ3rCU08V6Dcs5uGfM7HwjNXFadYwJv92c9R?=
 =?us-ascii?Q?x1o15ZYGZ3J5BlvD/Mh13e6yR0HC9wQUGmIQsJOjh+z7i/bk7mq/oCLdQaT6?=
 =?us-ascii?Q?hPTUN3IX2+rBTuutKSpF5h/Su7cK9qful/j+jhIa/t1Qu+lb4Wz8ZMA6Tvkw?=
 =?us-ascii?Q?9rECweBN9dHCwt4qnON/6sTHt/I8IFFg92chZqBu4DxQTewUWEXrNLmFel5m?=
 =?us-ascii?Q?wl6gceIVv2lpcyEzMFbpnYaAs1uJ5s8XWr6wAjJF436k/gEav5MogkxsQXuX?=
 =?us-ascii?Q?XBYYS4upCWr+0qgfFnb5JvcVIozjLAmiHJs4ISY0in0w9Xdgawb/COBbtVjO?=
 =?us-ascii?Q?Ng3vCW0/AiITfhRFcqAweezYg74BcFVT6s3cp3pOv1IuCP69w2zTIuWmz+jg?=
 =?us-ascii?Q?v6dlhl0HTqq6ybYmduCglfelEYv8IXcA1TEHdJiFOLehYmO6CxEclFXSOQrB?=
 =?us-ascii?Q?qfbtGS2JpZ9uBKJTYBylyXBhCcfxB1Q26hbSxhZUUOhFrObd3ktSz7SF1fhD?=
 =?us-ascii?Q?HrSh29Jn67hB3ODNQrqtTX3Ls/uNq9LfH4PLG9bwk8dpijDyA7c4os4viMqT?=
 =?us-ascii?Q?XA22tG7XCMOZNBNc4oYoPnKgdor+vbse4KOKr0nVkN8lyAnWoEtg2yhlmOrA?=
 =?us-ascii?Q?cyfklsB0kbAvGo4HrTjrXTbcsL1RjI7ra6vEcLAP2rZ1NfA6Vdi5BW6wauAD?=
 =?us-ascii?Q?QumXL9QjHoPLp1NFyQmhL6CRHR0CXzTuAU6wwGWKXp8+FSmyzZYAhwHnYM/E?=
 =?us-ascii?Q?D5v+jvmkSV35wu2JyJgENlcOHzY3RJyX9MbLu703vWnJTmnL/4j1a1Wd6Nzu?=
 =?us-ascii?Q?ufuNfwotDYaifP7dOOYQspjOAxYJrtDvzVt/3iapLJ8grRWKZkjaLAAWdKcs?=
 =?us-ascii?Q?jrEgpTcYRKz3PZXCUFWWkUiXqc9BZ5RdD24DkIBIlM03vIC5rnyz2Zy2fjjF?=
 =?us-ascii?Q?mYYOgPFrPeXOfjU6MgJQT/DU?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c0c87086-80ee-48e0-14a9-08d982737a23
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1001MB2365.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Sep 2021 11:31:13.7231
 (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: S38igHaCOCGPHc1A9APvDLRrUjyMMcvkKQ4s3WTUmGOG+pmlaYT736AkRopQvOXPTN1LspmqWJ+wlfHk4CtT89YdqzrUq2Ptv0KL5/q2cLE=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR1001MB2205
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10120 signatures=668682
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 phishscore=0 adultscore=0
 mlxscore=0 malwarescore=0 suspectscore=0 bulkscore=0 mlxlogscore=801
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2109230001
 definitions=main-2109280065
X-Proofpoint-ORIG-GUID: E4nu5C1DBWYEKbt4zEx9rC18XeIuIa52
X-Proofpoint-GUID: E4nu5C1DBWYEKbt4zEx9rC18XeIuIa52
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Tue, Sep 28, 2021 at 02:09:06PM +0300, Pavel Skripkin wrote:
> On 9/28/21 14:06, Pavel Skripkin wrote:
> > > It's not actually the same.  The state has to be set before the
> > > device_register() or there is still a leak.
> > > 
> > Ah, I see... I forgot to handle possible device_register() error. Will
> > send v2 soon, thank you
> > 
> > 
> > 
> Wait... Yanfei's patch is already applied to net tree and if I understand
> correctly, calling put_device() 2 times will cause UAF or smth else.
> 

Yes.  It causes a UAF.

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.

Anyway, your patch is required and the __devm_mdiobus_register()
function has leaks as well.  And perhaps there are more bugs we have not
discovered.

regards,
dan carpenter