All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sunil Mushran <sunil.mushran@oracle.com>
To: Joel Becker <joel.becker@oracle.com>
Cc: hch@infradead.org, viro@zeniv.linux.org.uk,
	ocfs2-devel@oss.oracle.com, linux-fsdevel@kernel.org
Subject: [Ocfs2-devel] [PATCH 2/2] ocfs2: Use MAY_CREATE in ocfs2_permission()
Date: Wed, 02 Sep 2009 18:45:47 -0700	[thread overview]
Message-ID: <4A9F1FCB.5020406@oracle.com> (raw)
In-Reply-To: <1251940014-20963-3-git-send-email-joel.becker@oracle.com>

Joel Becker wrote:
> ocfs2 has a problem with open(O_CREAT|O_EXCL).  Once you've created the
> file, you can't restart the open(), because O_CREAT|O_EXCL will trigger
> -EEXIST.
>
> The problem is that ocfs2 is catching the signal ->permission(), called
> by may_open().  This happens after ->create() has successfully created
> the file.  ocfs2_permission() has to get a cluster lock, and this is
> what can be interrupted by a signal.  Now, obviously we want to block
> signals in the O_CREAT|O_EXCL case, but ocfs2_permission() has no way of
> knowing it just got called from open_namei_create().
>
> We key on the MAY_CREATE flag passed to permission to block signals.
>
> Signed-off-by: Joel Becker <joel.becker@oracle.com>
> ---
>  fs/ocfs2/file.c |   13 +++++++++++++
>  1 files changed, 13 insertions(+), 0 deletions(-)
>
> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> index aa501d3..508a2db 100644
> --- a/fs/ocfs2/file.c
> +++ b/fs/ocfs2/file.c
> @@ -1095,9 +1095,18 @@ bail:
>  int ocfs2_permission(struct inode *inode, int mask)
>  {
>  	int ret;
> +	sigset_t oldset;
>  
>  	mlog_entry_void();
>  
> +	/*
> +	 * If this inode was just created by open(O_CREAT|O_EXCL), we
> +	 * can't allow signal restarting.  So we need to block signals
> +	 * around the cluster locking.
> +	 */
> +	if (mask & MAY_CREATE)
> +		ocfs2_block_signals(&oldset);
> +
>  	ret = ocfs2_inode_lock(inode, NULL, 0);
>  	if (ret) {
>  		if (ret != -ENOENT)
> @@ -1108,6 +1117,10 @@ int ocfs2_permission(struct inode *inode, int mask)
>  	ret = generic_permission(inode, mask, ocfs2_check_acl);
>  
>  	ocfs2_inode_unlock(inode, 0);
> +
> +	if (mask & MAY_CREATE)
> +		ocfs2_unblock_signals(&oldset);
> +
>  out:
>  	mlog_exit(ret);
>  	return ret;
>   

Maybe I am missing something but shouldn't we be unblocking the signal
after the out label.

WARNING: multiple messages have this Message-ID (diff)
From: Sunil Mushran <sunil.mushran@oracle.com>
To: Joel Becker <joel.becker@oracle.com>
Cc: hch@infradead.org, viro@zeniv.linux.org.uk,
	ocfs2-devel@oss.oracle.com, linux-fsdevel@kernel.org
Subject: Re: [PATCH 2/2] ocfs2: Use MAY_CREATE in ocfs2_permission()
Date: Wed, 02 Sep 2009 18:45:47 -0700	[thread overview]
Message-ID: <4A9F1FCB.5020406@oracle.com> (raw)
In-Reply-To: <1251940014-20963-3-git-send-email-joel.becker@oracle.com>

Joel Becker wrote:
> ocfs2 has a problem with open(O_CREAT|O_EXCL).  Once you've created the
> file, you can't restart the open(), because O_CREAT|O_EXCL will trigger
> -EEXIST.
>
> The problem is that ocfs2 is catching the signal ->permission(), called
> by may_open().  This happens after ->create() has successfully created
> the file.  ocfs2_permission() has to get a cluster lock, and this is
> what can be interrupted by a signal.  Now, obviously we want to block
> signals in the O_CREAT|O_EXCL case, but ocfs2_permission() has no way of
> knowing it just got called from open_namei_create().
>
> We key on the MAY_CREATE flag passed to permission to block signals.
>
> Signed-off-by: Joel Becker <joel.becker@oracle.com>
> ---
>  fs/ocfs2/file.c |   13 +++++++++++++
>  1 files changed, 13 insertions(+), 0 deletions(-)
>
> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> index aa501d3..508a2db 100644
> --- a/fs/ocfs2/file.c
> +++ b/fs/ocfs2/file.c
> @@ -1095,9 +1095,18 @@ bail:
>  int ocfs2_permission(struct inode *inode, int mask)
>  {
>  	int ret;
> +	sigset_t oldset;
>  
>  	mlog_entry_void();
>  
> +	/*
> +	 * If this inode was just created by open(O_CREAT|O_EXCL), we
> +	 * can't allow signal restarting.  So we need to block signals
> +	 * around the cluster locking.
> +	 */
> +	if (mask & MAY_CREATE)
> +		ocfs2_block_signals(&oldset);
> +
>  	ret = ocfs2_inode_lock(inode, NULL, 0);
>  	if (ret) {
>  		if (ret != -ENOENT)
> @@ -1108,6 +1117,10 @@ int ocfs2_permission(struct inode *inode, int mask)
>  	ret = generic_permission(inode, mask, ocfs2_check_acl);
>  
>  	ocfs2_inode_unlock(inode, 0);
> +
> +	if (mask & MAY_CREATE)
> +		ocfs2_unblock_signals(&oldset);
> +
>  out:
>  	mlog_exit(ret);
>  	return ret;
>   

Maybe I am missing something but shouldn't we be unblocking the signal
after the out label.

  reply	other threads:[~2009-09-03  1:45 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-09-03  1:06 [Ocfs2-devel] [PATCH 0/2] [RFC] Adding the MAY_CREATE flag to ->permission() Joel Becker
2009-09-03  1:06 ` Joel Becker
2009-09-03  1:06 ` [Ocfs2-devel] [PATCH 1/2] vfs: Add MAY_CREATE to the permission() flags Joel Becker
2009-09-03  1:06   ` Joel Becker
2009-09-03  1:06 ` [Ocfs2-devel] [PATCH 2/2] ocfs2: Use MAY_CREATE in ocfs2_permission() Joel Becker
2009-09-03  1:06   ` Joel Becker
2009-09-03  1:45   ` Sunil Mushran [this message]
2009-09-03  1:45     ` Sunil Mushran
2009-09-03 19:12     ` [Ocfs2-devel] " Joel Becker
2009-09-03 19:12       ` Joel Becker
  -- strict thread matches above, loose matches on Subject: below --
2009-10-14  9:57 [Ocfs2-devel] [PATCH 0/2] [RFC] Adding the MAY_CREATE flag to ->permission() Joel Becker
2009-10-14  9:57 ` [Ocfs2-devel] [PATCH 2/2] ocfs2: Use MAY_CREATE in ocfs2_permission() Joel Becker

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=4A9F1FCB.5020406@oracle.com \
    --to=sunil.mushran@oracle.com \
    --cc=hch@infradead.org \
    --cc=joel.becker@oracle.com \
    --cc=linux-fsdevel@kernel.org \
    --cc=ocfs2-devel@oss.oracle.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.