Received: by 2002:a05:6a10:5bc5:0:0:0:0 with SMTP id os5csp2621930pxb; Sun, 17 Oct 2021 20:46:21 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxYiHSjYcmAow6uhc1gtwuGFMHaYGeYqwKJklXnPzy2ALVts63/Zy9wiQ7QHOBZJhb3clk/ X-Received: by 2002:a63:b20c:: with SMTP id x12mr21545013pge.10.1634528780928; Sun, 17 Oct 2021 20:46:20 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1634528780; cv=none; d=google.com; s=arc-20160816; b=BkuZwSURTwbr9Sn5IXuo31Jppr+lfYhrVJcaJ2kchJykxQdXQ2vHb4WMW2ki4e5seQ EGFB2wCFfcFVkGgj1oR2rxVB04brWQxpvRNPNydAeXnDIE461Xl4QIqvNVImcSGLTLtD BZQni/XwKsWYlsfJE7DTIRPPbDQvSJDpAIuLaQ25tqNvAQ+UdIvZg8IghiXGueaLpjs4 VT93AEZdZFZjHRBG6b8CysOejZDe/w1YP6v9Bkc+bD20lfkzKZ6hBGUnC9py62zn4FZW Zf9m9hJnRSlwRr2+k1nKYPxFWB8mSUAANQS679Tvpa6qdq/wIwj5U+fg9mKidhTUwExI lBow== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-disposition:mime-version :references:message-id:subject:cc:to:from:date:dkim-signature; bh=AwYbTVxyEEqnVUpaTtiLQuoVyA1WDJHBXgFoiEkkYOs=; b=EWqPkpAqfIZ9+fW9NZi1s1CFMkXSWsi67Es5Xomcg2T1n+hVwiPHsqfgJ9+Seo3x/u xTyn0mKT4mnsDX2L+h9o8cH7M+2PN4PbNCiwKz69VPQeFuGhfBQTB9CS0bOxe3NV/e9s bPSyOU7TO0u7rHEcQFXTBdsTiUPIQqdd9f4kg9++dIT+SbDfDE1cNqtDWMhzqMLqLpeG fkuNqHb+AEH7K8/SciqyBucgeWc70+tcXL9SKJbYF6K7TQI3YRPo+toVcpYwX7Cioi/t +EnlvibFEMmFGw0z4PJaV3sgCIbCznstOmrxd+GPHDG6lF3JSj2QXATMdYcarKjm0YeN 3ctg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linuxfoundation.org header.s=korg header.b=jdhzIg9P; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linuxfoundation.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id s20si3400853pjn.20.2021.10.17.20.46.09; Sun, 17 Oct 2021 20:46:20 -0700 (PDT) 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; Authentication-Results: mx.google.com; dkim=pass header.i=@linuxfoundation.org header.s=korg header.b=jdhzIg9P; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linuxfoundation.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1343850AbhJQOm3 (ORCPT + 98 others); Sun, 17 Oct 2021 10:42:29 -0400 Received: from mail.kernel.org ([198.145.29.99]:59736 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237972AbhJQOm0 (ORCPT ); Sun, 17 Oct 2021 10:42:26 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id 519B16103B; Sun, 17 Oct 2021 14:40:16 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1634481617; bh=yFokzv3wpJKEz8i/ueDpiyf7dalfoEdoP328kL01z5Y=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=jdhzIg9PWvJ9gB7pq+4GqWniCadchsF7B2whNVeipbDqyzKEoqqlUrKKyUdfO/PIZ mhAKh74yT6zwxTrg+a4WlpDVmam2x+I3Sc3YEUq0M4Pcz7jNKVdUKh6gV4iqV4Qu4h 8FTROEnuqqDSFeEL7Zoyy2+SNflEKDYIQHhlAlQ4= Date: Sun, 17 Oct 2021 16:40:14 +0200 From: Greg KH To: Alexandre Belloni Cc: William Breathitt Gray , jic23@kernel.org, linux-stm32@st-md-mailman.stormreply.com, kernel@pengutronix.de, a.fatoum@pengutronix.de, kamel.bouhara@bootlin.com, gwendal@chromium.org, david@lechnology.com, linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, syednwaris@gmail.com, patrick.havelange@essensium.com, fabrice.gasnier@st.com, mcoquelin.stm32@gmail.com, alexandre.torgue@st.com, o.rempel@pengutronix.de, jarkko.nikula@linux.intel.com, Dan Carpenter Subject: Re: [PATCH v17 2/9] counter: Add character device interface Message-ID: References: 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-kernel@vger.kernel.org On Sun, Oct 17, 2021 at 04:02:42PM +0200, Alexandre Belloni wrote: > On 17/10/2021 15:50:11+0200, Greg KH wrote: > > Note, review of this now that it has been submitted in a pull request to > > me, sorry I missed this previously... > > > > On Wed, Sep 29, 2021 at 12:15:59PM +0900, William Breathitt Gray wrote: > > > +static int counter_chrdev_open(struct inode *inode, struct file *filp) > > > +{ > > > + struct counter_device *const counter = container_of(inode->i_cdev, > > > + typeof(*counter), > > > + chrdev); > > > + > > > + /* Ensure chrdev is not opened more than 1 at a time */ > > > + if (!atomic_add_unless(&counter->chrdev_lock, 1, 1)) > > > + return -EBUSY; > > > > I understand the feeling that you wish to stop userspace from doing > > this, but really, it does not work. Eventhough you are doing this > > correctly (you should see all the other attempts at doing this), you are > > not preventing userspace from having multiple processes access this > > device node at the same time, so please, don't even attempt to stop this > > from happening. > > > > So you can drop the atomic "lock" you have here, it's not needed at all. > > > > Could you elaborate a bit here because we've had a similar thing in the > RTC subsystem: > > https://elixir.bootlin.com/linux/latest/source/drivers/rtc/dev.c#L28 Yeah, that too will not work :( Note, it does stop open from being called from different processes, but think of the following sequence of userspace calls: open() fork/exec() both processes access the file descriptor or passing a fd across a socket? Or duplicating the file descriptor and sending it to a different task (like across a socket or many other IPC ways)? Once userspace has a file descriptor, all bets are off as to where it goes and what it does with it. There's no need to try to save userspace from itself by preventing multiple opens when really, it doesn't stop anyone who really wants to do this. If userspace does do multiple read/writes from different threads / processes / whatever on the same file descriptor, it gets to keep the pieces of the mess it causes. It's not the kernel's job to try to "protect" userspace from itself here. Look at serial/tty connections as one example of this always being the case. Does that help? > And it would mean I can remove rtc->flags completely. I think you can do that. thanks, greg k-h