linux-kernel - Re: [PATCH 14/18] virtiofs: Add a fuse_iqueue operation to put() reference

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190906133555.GC22083@redhat.com>
Date:   Fri, 6 Sep 2019 09:35:55 -0400
From:   Vivek Goyal <vgoyal@...hat.com>
To:     Stefan Hajnoczi <stefanha@...hat.com>
Cc:     linux-fsdevel@...r.kernel.org,
        virtualization@...ts.linux-foundation.org, miklos@...redi.hu,
        linux-kernel@...r.kernel.org, virtio-fs@...hat.com,
        dgilbert@...hat.com, mst@...hat.com
Subject: Re: [PATCH 14/18] virtiofs: Add a fuse_iqueue operation to put()
 reference

On Fri, Sep 06, 2019 at 01:00:09PM +0100, Stefan Hajnoczi wrote:
> On Thu, Sep 05, 2019 at 03:48:55PM -0400, Vivek Goyal wrote:
> > diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> > index 85e2dcad68c1..04e2c000d63f 100644
> > --- a/fs/fuse/fuse_i.h
> > +++ b/fs/fuse/fuse_i.h
> > @@ -479,6 +479,11 @@ struct fuse_iqueue_ops {
> >  	 */
> >  	void (*wake_pending_and_unlock)(struct fuse_iqueue *fiq)
> >  		__releases(fiq->waitq.lock);
> > +
> > +	/**
> > +	 * Put a reference on fiq_priv.
> 
> I'm a bit confused about fiq->priv's role in this.  The callback takes
> struct fuse_iqueue *fiq as the argument, not void *priv, so it could
> theoretically do more than just release priv.
> 
> I think one of the following would be clearer:
> 
>  /**
>   * Drop a reference to fiq->priv.
>   */
>  void (*put_priv)(void *priv);
> 
> Or:
> 
>  /**
>   * Clean up when fuse_iqueue is destroyed.
>   */
>  void (*release)(struct fuse_iqueue *fiq);
> 
> In the second case fuse_conn_put() shouldn't check fiq->priv.

Hi Stefan,

I like using ->release() method sounds better. Here is the updated patch.
This also changes the next patch, so will post that as well.

Thanks
Vivek

Subject: virtiofs: Add a fuse_iqueue operation to put() reference

Soon I will make virtio_fs object reference counted, where reference will
be taken by device as well as by fuse_conn (fuse_conn->fuse_iqueue->fiq_priv).

When fuse_connection is going away, it should put its reference on virtio_fs
object.

So add a fuse_iqueue method which can be used to call into virtio_fs to
put the reference on the object (fiq_priv).

Signed-off-by: Vivek Goyal <vgoyal@...hat.com>
---
 fs/fuse/fuse_i.h |    5 +++++
 fs/fuse/inode.c  |    4 ++++
 2 files changed, 9 insertions(+)

Index: rhvgoyal-linux-fuse/fs/fuse/fuse_i.h
===================================================================
--- rhvgoyal-linux-fuse.orig/fs/fuse/fuse_i.h	2019-09-06 08:46:41.846086544 -0400
+++ rhvgoyal-linux-fuse/fs/fuse/fuse_i.h	2019-09-06 09:24:32.752245246 -0400
@@ -479,6 +479,11 @@ struct fuse_iqueue_ops {
 	 */
 	void (*wake_pending_and_unlock)(struct fuse_iqueue *fiq)
 		__releases(fiq->waitq.lock);
+
+	/**
+	 * Cleanup up when fuse_iqueue is destroyed
+	 */
+	void (*release)(struct fuse_iqueue *fiq);
 };
 
 /** /dev/fuse input queue operations */
Index: rhvgoyal-linux-fuse/fs/fuse/inode.c
===================================================================
--- rhvgoyal-linux-fuse.orig/fs/fuse/inode.c	2019-09-06 08:46:41.847086544 -0400
+++ rhvgoyal-linux-fuse/fs/fuse/inode.c	2019-09-06 09:24:32.754245246 -0400
@@ -631,8 +631,12 @@ EXPORT_SYMBOL_GPL(fuse_conn_init);
 void fuse_conn_put(struct fuse_conn *fc)
 {
 	if (refcount_dec_and_test(&fc->count)) {
+		struct fuse_iqueue *fiq = &fc->iq;
+
 		if (fc->destroy_req)
 			fuse_request_free(fc->destroy_req);
+		if (fiq->ops->release)
+			fiq->ops->release(fiq);
 		put_pid_ns(fc->pid_ns);
 		put_user_ns(fc->user_ns);
 		fc->release(fc);