From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 30B9BC4167B for ; Tue, 29 Nov 2022 23:57:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229690AbiK2X5y (ORCPT ); Tue, 29 Nov 2022 18:57:54 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46832 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230293AbiK2X5l (ORCPT ); Tue, 29 Nov 2022 18:57:41 -0500 Received: from mail-pf1-x42a.google.com (mail-pf1-x42a.google.com [IPv6:2607:f8b0:4864:20::42a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9174A25A; Tue, 29 Nov 2022 15:57:33 -0800 (PST) Received: by mail-pf1-x42a.google.com with SMTP id o1so10722613pfp.12; Tue, 29 Nov 2022 15:57:33 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=FYMDmlsvjozIL5WOmq1zpgn4QhNwIkK2G4uNnYQOjuk=; b=hXTSncZ/do9WkQnengsdG54Eb/LOE5zQ80xuJXTxQeKqwEd+eK8zetZRndZwfpNqXY NolX03Q9qHqsv+jK7aZ95ph4Cg5J54bYlE+z5x1krEnCxOGbmSY7qBw8pstXBMbX6xCD CnhVyMLGMeT9SzXjoiS34wWK0yA+cv7mx/mnsOaZjWPNi7d7w0vif3UzkXuJmorFRL7b Ng2rkfTMoxfju98F3/+pIm7mNS4PVUJJGDOUQcTmvNcSkslsn6S2LH5NcjTvrIpX+dkS 69QeuYgAX7g+LOoAF6F+x3rPYZ2mvCewZ3PxipdaRXkjmNpacfbSzgWcnZ/6o2xAac6k jDEQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=FYMDmlsvjozIL5WOmq1zpgn4QhNwIkK2G4uNnYQOjuk=; b=fzOrYHceSShEuPMyalki4s+guFicsJkc1YstEAL1wH1fMZwrzX+7bEftuVXgskutYx DBI3vUyBS2sF8k/G+AMm+yq/gvWSpZjCiVJjN3LskOIFH1q4n/3bnBAj0+JZcEliR5fw M0etecyueoFOJTe6smcJRZsWAJAeJxH3gBlJmflwaIiSyzqEk2RmSKc2+fer6owwa22g U20eU3zbKRqPse+fP+5UCyytngh6NSTqRHroVsJvnJ+AI+s2QFNdpCcH4U7MFI8tq9Yf U+5M1b4ooryM90UdKqKSpmvaSCncctNS9OiTRKl1BPIu0B0D6yWCVqV5vPwZanKn4TH0 9PKQ== X-Gm-Message-State: ANoB5plM2A76G/iNjRfY4C+QlyDHNO6odyCQyDoT9rDUnoGQrIoFdEm9 EHqj66OBPXMvzxT78X4EYZI= X-Google-Smtp-Source: AA0mqf5vn97HJ2AWPh8OLCPTXpCymTiMwpM6M4PRuB27mCC2bgSz7yiGw1Po5rWFTmtvLnaOQTx2sg== X-Received: by 2002:a63:5a53:0:b0:477:ae2f:3292 with SMTP id k19-20020a635a53000000b00477ae2f3292mr30795903pgm.267.1669766252976; Tue, 29 Nov 2022 15:57:32 -0800 (PST) Received: from sol (110-174-14-241.tpgi.com.au. [110.174.14.241]) by smtp.gmail.com with ESMTPSA id i4-20020a626d04000000b0056d98e359a5sm35622pfc.165.2022.11.29.15.57.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 29 Nov 2022 15:57:32 -0800 (PST) Date: Wed, 30 Nov 2022 07:57:26 +0800 From: Kent Gibson To: Bartosz Golaszewski Cc: Andy Shevchenko , Linus Walleij , linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org, Bartosz Golaszewski Subject: Re: [PATCH v3 2/2] gpiolib: protect the GPIO device against being dropped while in use by user-space Message-ID: References: <20221129123553.353410-1-brgl@bgdev.pl> <20221129123553.353410-3-brgl@bgdev.pl> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-gpio@vger.kernel.org On Tue, Nov 29, 2022 at 06:53:26PM +0200, Andy Shevchenko wrote: > On Tue, Nov 29, 2022 at 01:35:53PM +0100, Bartosz Golaszewski wrote: > > From: Bartosz Golaszewski > > > > While any of the GPIO cdev syscalls is in progress, the kernel can call > > gpiochip_remove() (for instance, when a USB GPIO expander is disconnected) > > which will set gdev->chip to NULL after which any subsequent access will > > cause a crash. > > > > To avoid that: use an RW-semaphore in which the syscalls take it for > > reading (so that we don't needlessly prohibit the user-space from calling > > syscalls simultaneously) while gpiochip_remove() takes it for writing so > > that it can only happen once all syscalls return. > > ... > > I would do > > typedef __poll_t (*poll_fn)(struct file *, struct poll_table_struct *); > > and so on and use that one in the respective parameters. > > BUT. Since it's a fix, up to you which one to choose. > FWIW, the typedef looks cleaner to me too. > > +static __poll_t call_poll_locked(struct file *file, > > + struct poll_table_struct *wait, > > + struct gpio_device *gdev, > > + __poll_t (*func)(struct file *, > > + struct poll_table_struct *)) > > +{ > > + __poll_t ret; > > + > > + down_read(&gdev->sem); > > + ret = func(file, wait); > > + up_read(&gdev->sem); > > + > > + return ret; > > +} > > ... > > > + down_write(&gdev->sem); > > + Blank line? > Agreed. > > /* FIXME: should the legacy sysfs handling be moved to gpio_device? */ > > gpiochip_sysfs_unregister(gdev); > > gpiochip_free_hogs(gc); > > ... > > > gcdev_unregister(gdev); > > + Blank line ? > Disagree with this one though. The comment prior to the gcdev_unregister() appears to apply to the block, so the following lines should remain grouped. Other than those nits, the series looks good to me. Reviewed-by: Kent Gibson Cheers, Kent. > > + up_write(&gdev->sem); > > put_device(&gdev->dev); > > -- > With Best Regards, > Andy Shevchenko > >