linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jan Kara <jack@suse.cz>
To: Alexander Lochmann <alexander.lochmann@tu-dortmund.de>
Cc: Jan Kara <jack@suse.cz>,
	viro@zeniv.linux.org.uk, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Horst Schirmeier <horst.schirmeier@tu-dortmund.de>,
	linux-block@vger.kernel.org, Jens Axboe <axboe@kernel.dk>
Subject: Re: [PATCH] Fix sync. in inode_has_no_xattr()
Date: Fri, 7 Dec 2018 12:14:26 +0100	[thread overview]
Message-ID: <20181207111426.GG13008@quack2.suse.cz> (raw)
In-Reply-To: <c4f0d66f-7cad-c3b4-79a6-ced14b9c7cc8@tu-dortmund.de>

On Fri 07-12-18 11:24:47, Alexander Lochmann wrote:
> Am 05.12.18 um 16:32 schrieb Jan Kara:
> > 
> > Thinking more about this I'm not sure if this is actually the right
> > solution. Because for example the write(2) can set S_NOSEC flag wrongly
> > when it would race with chmod adding SUID bit. So probably we rather need
> > to acquire i_rwsem in blkdev_write_iter() if file does not have S_NOSEC set
> > (we don't want to acquire it unconditionally as that would heavily impact
> > scalability of block device writes).
> > 
> > 								Honza
> > 
> Trying to implement your suggestion, I'm not sure which inode to use:
> In blkdev_write_iter() there is the "bd_inode = bdev_file_inode(file)".
> file_remove_privs() uses "inode = file_inode(file)" as a parameter for
> inode_has_no_xattr().
> So, do file->f_mapping->host and f->f_inode refer to the identical inode?

Ah, that's a good question and I forgot to warn you. Sorry for that and my
respect for noticing this yourself :). For block devices f->f_inode refers
to the device inode in the filesystem (i.e., where xattrs are attached to,
permissions are stored, etc.).  file->f_mapping->host refers to the inode
carrying data - this split is needed so that if there are more instances of
device inode in the system for the same block device, they see data
coherently. So you need to use file_inode(file) for the locking. Thanks!

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

      reply	other threads:[~2018-12-07 11:14 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <13eaa1c6-def5-afad-89eb-6f149db90684@tu-dortmund.de>
2018-12-05 15:32 ` [PATCH] Fix sync. in inode_has_no_xattr() Jan Kara
2018-12-07 10:24   ` Alexander Lochmann
2018-12-07 11:14     ` Jan Kara [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181207111426.GG13008@quack2.suse.cz \
    --to=jack@suse.cz \
    --cc=alexander.lochmann@tu-dortmund.de \
    --cc=axboe@kernel.dk \
    --cc=horst.schirmeier@tu-dortmund.de \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).