linux-kernel - Re: [PATCH] btrfs: explicitly set control file's private

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: <20150324140155.GU20767@twin.jikos.cz>
Date:	Tue, 24 Mar 2015 15:01:55 +0100
From:	David Sterba <dsterba@...e.cz>
To:	Tom Van Braeckel <tomvanbraeckel@...il.com>
Cc:	clm@...com, jbacik@...com, dsterba@...e.cz,
	linux-btrfs@...r.kernel.org, linux-kernel@...r.kernel.org,
	Martin Kepplinger <martink@...teo.de>
Subject: Re: [PATCH] btrfs: explicitly set control file's private_data

On Mon, Mar 23, 2015 at 06:34:13PM +0100, Tom Van Braeckel wrote:
> The private_data member of the Btrfs control device file
> (/dev/btrfs-control) is used to hold the current transaction and needs
> to be initialized to NULL to signify that no transaction is in progress.
> 
> We explicitly set the control file's private_data to NULL to be
> independent of whatever value the misc subsystem initializes it to.
> 
> Backstory:
> ----------
> 
> The misc subsystem (which is used by /dev/btrfs-control) initializes
> a file's private_data to point to the misc device when a driver has
> registered a custom open file operation and initializes it to NULL
> when a custom open file operation has *not* been provided.
> 
> This subtle quirk is confusing, to the point where kernel code registers
> *empty* file open operations to have private_data point to the misc
> device structure.
> 
> And it leads to bugs, where the addition or removal of a custom open
> file operation surprisingly changes the initial contents of a file's
> private_data structure.
> 
> To simplify things in the misc subsystem, a patch [1] has been proposed
> to *always* set private_data to point to the misc device instead of
> only doing this when a custom open file operation has been registered.
> 
> But before we can fix this in the misc subsystem itself, we need to
> modify the (few) drivers that rely on this very subtle behavior.
> 
> [1] https://lkml.org/lkml/2014/12/4/939
> 
> Signed-off-by: Martin Kepplinger <martink@...teo.de>
> Signed-off-by: Tom Van Braeckel <tomvanbraeckel@...il.com>

Thanks for the explanation.

Acked-by: David Sterba <dsterba@...e.cz>

> +static int btrfs_control_open(struct inode *inode, struct file *file)
> +{
> +	/* The control file's private_data is used to hold the
> +	 * transaction when it is started and is used to keep
> +	 * track of whether a transaction is already in progress.
> +	 */

That's not the common comment style (newline after /* ) but I'm never
sure whether I should nitpick about such things or just say yes to
fixes.

> +	file->private_data = NULL;
> +	return 0;
> +}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/