Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Peter Chen <peter.chen@nxp.com>
To:     Pavel Machek <pavel@denx.de>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CC:     "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "stable@vger.kernel.org" <stable@vger.kernel.org>,
        Felipe Balbi <felipe.balbi@linux.intel.com>,
        Sasha Levin <sashal@kernel.org>
Subject: RE: [PATCH 4.19 093/125] usb: gadget: configfs: fix concurrent issue
 between composite APIs
Thread-Topic: [PATCH 4.19 093/125] usb: gadget: configfs: fix concurrent issue
 between composite APIs
Thread-Index: AQHVmMAk0+NCuNSe+0SwjLKirLuN5KeJIN4AgAEXLnA=
Date:   Thu, 14 Nov 2019 06:44:58 +0000
Message-ID: <VI1PR04MB532778F3728AB7465D0892F38B710@VI1PR04MB5327.eurprd04.prod.outlook.com>
References: <20191111181438.945353076@linuxfoundation.org>
 <20191111181452.322265396@linuxfoundation.org>
 <20191113134935.GA20980@duo.ucw.cz>
In-Reply-To: <20191113134935.GA20980@duo.ucw.cz>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: nxp.com does not designate
 permitted sender hosts)
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 76415925-ab42-4310-cc05-08d768ce2a7e
X-MS-Exchange-CrossTenant-originalarrivaltime: 14 Nov 2019 06:44:58.0739
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: C+JMv2OFLWx8cgrzEUx9sYHngxwydcfIAKKrUANT9vLZuzOEXwzwbiBeQps/7P58pjlMwu6iYruy/ZXplBJqSg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR04MB5072
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

=20
> >
> > We meet several NULL pointer issues if configfs_composite_unbind and
> > composite_setup (or composite_disconnect) are running together.
> > These issues occur when do the function switch stress test, the
> > configfs_compsoite_unbind is called from user mode by echo "" to
> > /sys/../UDC entry, and meanwhile, the setup interrupt or disconnect
> > interrupt occurs by hardware. The composite_setup
>=20
> > +++ b/drivers/usb/gadget/configfs.c
> > @@ -61,6 +61,8 @@ struct gadget_info {
> >  	bool use_os_desc;
> >  	char b_vendor_code;
> >  	char qw_sign[OS_STRING_QW_SIGN_LEN];
> > +	spinlock_t spinlock;
> > +	bool unbind;
> >  };
> >
> >  static inline struct gadget_info *to_gadget_info(struct config_item
> > *item) @@ -1244,6 +1246,7 @@ static int configfs_composite_bind(struct
> usb_gadget *gadget,
> >  	int				ret;
> >
> >  	/* the gi->lock is hold by the caller */
> > +	gi->unbind =3D 0;
> >  	cdev->gadget =3D gadget;
>=20
> Since variable is bool, I'd expect "=3D false" here?
>=20
> > +	unsigned long flags;
> >
> >  	/* the gi->lock is hold by the caller */
>=20
> "is held".
>=20
> >  	cdev =3D get_gadget_data(gadget);
> >  	gi =3D container_of(cdev, struct gadget_info, cdev);
> > +	spin_lock_irqsave(&gi->spinlock, flags);
> > +	gi->unbind =3D 1;
>=20
> =3D true;
>=20
> > +static int configfs_composite_setup(struct usb_gadget *gadget,
> > +		const struct usb_ctrlrequest *ctrl) {
> > +	struct usb_composite_dev *cdev;
> > +	struct gadget_info *gi;
> > +	unsigned long flags;
> > +	int ret;
> > +
> > +	cdev =3D get_gadget_data(gadget);
> > +	if (!cdev)
> > +		return 0;
> > +
> > +	gi =3D container_of(cdev, struct gadget_info, cdev);
> > +	spin_lock_irqsave(&gi->spinlock, flags);
> > +	cdev =3D get_gadget_data(gadget);
>=20
> cdev already contains required value, why get it second time? (If it need=
s to be
> done under lock, comment might be useful...)
>=20

Hi Pavel,

Thanks for comment.

The reason is the cdev may be set to NULL by configfs_composite_unbind thro=
ugh
another process, eg, the user wants to disable current USB gadget function,=
=20
this patch tries to fix such kinds of concurrent issue.

Peter
=20
=20