From mboxrd@z Thu Jan  1 00:00:00 1970
From: Christoph Hellwig <hch@infradead.org>
Date: Sun, 24 Aug 2014 08:58:58 -0700
Subject: [Cluster-devel] [PATCH 07/10] locks: define a lm_setup handler
	for leases
In-Reply-To: <1408804878-1331-8-git-send-email-jlayton@primarydata.com>
References: <1408804878-1331-1-git-send-email-jlayton@primarydata.com>
	<1408804878-1331-8-git-send-email-jlayton@primarydata.com>
Message-ID: <20140824155858.GF15908@infradead.org>
List-Id: <cluster-devel.redhat.com>
To: cluster-devel.redhat.com
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit

I like this change a lot!  But one caveat:

> +	/*
> +	 * Despite the fact that it's an int return function, __f_setown never
> +	 * returns an error. Just ignore any error return here, but spew a
> +	 * WARN_ON_ONCE in case this ever changes.
> +	 */
> +	WARN_ON_ONCE(__f_setown(filp, task_pid(current), PIDTYPE_PID, 0));

I don't think this is a good idea.  The errors in __f_setown come from
the security modules, and they could change easily.  If you can convince
the LSM people to change their file_set_fowner routine to return void
we could change __f_setown to return void as well and be done with it,
but without that this looks too dangerous.