Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   <Kumaravel.Thiagarajan@microchip.com>
To:     <harshit.m.mogalapalli@oracle.com>
CC:     <error27@gmail.com>, <dan.carpenter@linaro.org>,
        <kernel-janitors@vger.kernel.org>, <arnd@arndb.de>,
        <gregkh@linuxfoundation.org>, <linux-gpio@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <VaibhaavRam.TL@microchip.com>,
        <Jegadheesan.GopalManoharan@microchip.com>
Subject: RE: [PATCH] misc: microchip: pci1xxxx: Fix error handling in
 gp_aux_bus_probe()
Thread-Topic: [PATCH] misc: microchip: pci1xxxx: Fix error handling in
 gp_aux_bus_probe()
Thread-Index: AQHZiaaShOw8rdGJgk+41GVNKCKV4q9xD/ZggA+HExA=
Date:   Thu, 8 Jun 2023 06:57:03 +0000
Message-ID: <BN8PR11MB3668990F7108B177420FA3F2E950A@BN8PR11MB3668.namprd11.prod.outlook.com>
References: <20230518163333.1355445-1-harshit.m.mogalapalli@oracle.com>
 <BN8PR11MB366842432B7B3564E491AFC2E94A9@BN8PR11MB3668.namprd11.prod.outlook.com>
In-Reply-To: <BN8PR11MB366842432B7B3564E491AFC2E94A9@BN8PR11MB3668.namprd11.prod.outlook.com>
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?SZs4PhgWmOY2KsrHvVD/472HBNzsloiFlkdUPoPLBhQzKWi1UnB+QfldcpDp?=
 =?us-ascii?Q?l7t1CxUaKfLfJDC45RaXcGAEyJJOUUolFvVI+vgd+A03tkvF4RT0hkqyujZi?=
 =?us-ascii?Q?RkLgTqawL0gTRy9iRl/dRqCuLMTsBmHB1Hpol2+vKAnrvqxDXon2Kvj1Uhxc?=
 =?us-ascii?Q?WJwiN6CKHhzCop2R4N5HHDdldGFkzJaV8y8tgeYlhLsQvosesdBcdsr/SgsK?=
 =?us-ascii?Q?JrtLtM80YpCPLYC/OEks72hfPAwhJvi1cUkXf+sMuNgS9gl07JHiycLB9C/y?=
 =?us-ascii?Q?Y+USWnEClkIuAhnAMi8/ir1zGPjIL7TguaF1IpULMUmawgfdYTe4oFTFiowq?=
 =?us-ascii?Q?HEzg1iR6BzP4AxZe5sYSwEMvUUuLKHv1vhnNX+WU1FhTq38T5LTzmrmdqJik?=
 =?us-ascii?Q?zZPdoc9mK8aYncKEj/PAau6ItaaPYRr+g9IdLt6/bIsuCEvGkMy00oBGk352?=
 =?us-ascii?Q?koTYCVIx2wIuQbO8axpK1wWhbroxqjRb3TzZ+CvxlSROq8TT3G/E1OPbBgho?=
 =?us-ascii?Q?Qu0QkrRRuv8x2OX3FUX4/Or9SY9WlOUR6KiDfDL4rQJg+prmTlRatP3FT3//?=
 =?us-ascii?Q?WHtlIRvBDWYuGIqsyw/5J5dxet7NlwoTjeHThr3yRxh+5JBa+rCwEQdoOq8p?=
 =?us-ascii?Q?RAW96+9+mzUv0uDKIOFdakT7GeIHf3K2AyHA2Brbug5Vbvu5MPh+PrcuXqcE?=
 =?us-ascii?Q?pHehsU+AVqFda42f7FcDrnD7U42b1ObHZO6mpL5WTs4hbAvnP73VBjTF3ioc?=
 =?us-ascii?Q?2VhT7v0DaeTtybCA/xbYWYoQJhyIMez+prx0c8FaOzDq1URxYfLvVaIIED/j?=
 =?us-ascii?Q?sacmND0VAWoZEJRhKI8XNnR2qmPBPNl2Ldo4vA1qV5NmNWUCBAR8XMj4R55y?=
 =?us-ascii?Q?01mwUIEnsiUoDrN3Mt8bs4gFoIAirbudjPKZw8RNJyWIJWXYcdkaqD68M3qr?=
 =?us-ascii?Q?Q3MER7rcO3qoUvLViZVD1DOfvhTiWYNqAElXDcRbWort88KJ4pYsbJx/2AZo?=
 =?us-ascii?Q?tZZnP3OYQFyDG7fEoh+cjLlTay0q+CZ6iA1y3XRVFqBLiRjl+a/AZd5m5ntH?=
 =?us-ascii?Q?zLO3mcXcODA+r7JznhZ3TAbqpKtUUm0TQabOBKYHngBzPUrL7MOTDcalnNZ5?=
 =?us-ascii?Q?Z0KCxs8k0Zxmy2S8w2iUcdfCA/ynMhoxaGMFA/AjqpHC+YvhuTZMmoAF/vG4?=
 =?us-ascii?Q?qS/mjVwe0dU/OVxP+jJjmQYBSbX35JSRn3G0/y0kWdbyDl/GB2rOZA55Oh96?=
 =?us-ascii?Q?eKXBICsYABeQNcokwDNjCmaW4MhdPW6EltbDXajS//Yb3l/iLIqoixle1qlK?=
 =?us-ascii?Q?rhO9Ghl4d/NJ17TM9YzAO3+kZ97GUJRqFbcueTiYEVwiK5mzuMnfACBo1ZLh?=
 =?us-ascii?Q?q58xoYW7X+Btfb7NUEYGsiD7IcDaDA6TGUUVw+wuhOtOFQZi4OsvYqgZEZgR?=
 =?us-ascii?Q?08hOuaqWZO3nc1UHJNU+5A6lYmL2AGdYWb4kGcSojjkR0+SMZ2psoYNqKVqV?=
 =?us-ascii?Q?iQkPdB/TtmJf5+WBZ3BZ0gDgN2JyUujTBxy5uWjIYxCjlYN46+jKGepnyCB0?=
 =?us-ascii?Q?1RLjlI/dM2ZpDWsm9IDypd4XKF79uJVa6rMRpVNQ/spox+monA2zXAU9SgBn?=
 =?us-ascii?Q?dg3nlqCeZ5EdcaYGKQe7Yc4=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BN8PR11MB3668.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 57072c2b-0f76-47ed-bbbc-08db67ed906b
X-MS-Exchange-CrossTenant-originalarrivaltime: 08 Jun 2023 06:57:03.0730
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 3f4057f3-b418-4d4e-ba84-d55b4e897d88
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: uQuieTibQNCw4BDnZEyW4N1rKQcbsMv5lcb9yBRmaDcSxG+BGZ/QUolNcA0rjtKj7kOaluG1AC44w6lzPHsaf7fc5urcbIF8UMQa2+Ro0Mv8G92kIyBkyo7ix2JpPnuc
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB5389
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,SPF_HELO_PASS,
        SPF_NONE,T_SCC_BODY_TEXT_LINE autolearn=ham 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-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> -----Original Message-----
> From: Kumaravel Thiagarajan - I21417
> Sent: Monday, May 29, 2023 3:17 PM
> To: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
> Subject: RE: [PATCH] misc: microchip: pci1xxxx: Fix error handling in
> gp_aux_bus_probe()
>=20
> >
> > Smatch warns:
> >         drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c:73
> >         gp_aux_bus_probe() warn: missing unwind goto?
> >
> > Apart from above warning that smatch warns, we have other issues with
> > this function.
> >
> > 1. The call to auxiliary_device_add() needs a matching call to
> >    auxiliary_device_delete(). When memory allocation for
> >    "aux_bus->aux_device_wrapper[1]" fails we should also delete
> >    auxiliary device for "aux_device_wrapper[0]".
> > 2. In the error path when auxiliary_device_uninit() is called, it
> >    does trigger the release function --> gp_auxiliary_device_release(),
> >    this release function has the following:
> >
> >         ida_free(&gp_client_ida, aux_device_wrapper->aux_dev.id);
> >         kfree(aux_device_wrapper);
> >
> >    so few error paths have double frees. Eg: The goto label
> >    "err_aux_dev_add_0" first calls auxiliary_device_uninit() which also
> >    does an ida_free(), so when the control reaches "err_aux_dev_init_0"
> >    it will be a double free there.
> >
> > Re-write the error handling code. Clean up manually before the
> > auxiliary_device_init() calls succeed and use gotos to clean up after
> > they succeed. Also change the goto label names to follow freeing the
> > last thing to make it more readable.
> Thank You Harshit ! I reviewed your changes.
> But I need some time to apply the changes and test them.
> I will let you know as soon as we make progress.
> >
> > Fixes: 393fc2f5948f ("misc: microchip: pci1xxxx: load auxiliary bus
> > driver for the PIO function in the multi-function endpoint of pci1xxxx
> > device.")
> > Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
> > ---
> > Only compile tested, from static analysis.

Jegadheesan,

Please apply and test this patch - https://lore.kernel.org/all/202305181633=
33.1355445-1-harshit.m.mogalapalli@oracle.com/
It would fix some bugs in the pci1xxxx'x auxiliary bus driver's error handl=
ing path in the probe function.

Thank You.

Regards,
Kumar