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, 21 Apr 2020 12:28:29 -0700
From:   Dmitry Torokhov <dmitry.torokhov@gmail.com>
To:     Brendan Shanks <bshanks@codeweavers.com>
Cc:     linux-input@vger.kernel.org, rydberg@bitmath.org,
        linux-kernel@vger.kernel.org,
        Mathieu Maret <mathieu.maret@gmail.com>,
        mmaret@pixium-vision.com
Subject: Re: BUG: ff_effects lost after a fork
Message-ID: <20200421192829.GB125362@dtor-ws>
References: <20191127101008.GA327265@nuka.localdomain>
 <5404D7D5-47EF-4399-B0D9-B3C68A3D5895@codeweavers.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <5404D7D5-47EF-4399-B0D9-B3C68A3D5895@codeweavers.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, Apr 21, 2020 at 11:53:19AM -0700, Brendan Shanks wrote:
> 
> > On Nov 27, 2019, at 2:10 AM, Mathieu Maret <mathieu.maret@gmail.com>
> > wrote:
> > 
> > Hi,
> > 
> > I'm using evdev for vibrator interface.  I can register ff_effect
> > and play them.  But, if I do any kind of fork, all the effects are
> > flush and cannot be used.
> > 
> > You can find, below, an example of such a program.  From some trace
> > have put in the kernel, it's seems that at the end of the system()
> > call, evdev_flush get called.
> > 
> > evdev_flush() will call flush_effects() that will remove all the
> > registered effects.
> > 
> > I've only one device with vibrator and it's a imx6 4.1.15 kernel.
> > But code looks the same that in linus master that why I'm posting it
> > here. I hope that it will not waste people time
> > 
> 
> Hi everyone,
> 
> I’m also hitting this bug with games that use force-feedback steering
> wheels under Wine/Proton. It typically shows up as EVIOCSFF ioctls
> failing with EINVAL, since all the effects were unexpectedly flushed.
> 
> The problem is that input_ff_flush() is called whenever a file
> descriptor is closed, but there can be multiple descriptors open to
> the same file description (through fork(), dup(), etc).
> input_ff_flush() removes all effects added by that file description,
> which the users of the other descriptors certainly don't expect.
> 
> As for the fix, maybe fd_ops->flush() shouldn’t be implemented at all?
> In the current design, effects “belong” to a file description (a
> struct file *), not a descriptor. This seems sensible to me: a process
> could open a device, upload an effect, then fork(), and it makes sense
> that the child would have full control of the effects created by the
> parent. It seems to me like nothing should be done when a descriptor
> is closed, and input_ff_flush() should be called only when the whole
> struct file is released.
> 

Yes, I agree, we should drop setting fops->flush and move the code into
evdev_release(). This will ensure that the effects are stopped and
erased once file is closed, but will allow passing the descriptors
around.

Can I get a real patch for this please?

Thanks.

-- 
Dmitry