All of lore.kernel.org
 help / color / mirror / Atom feed
From: Colin Ian King <colin.king@canonical.com>
To: Jeff Mahoney <jeffm@suse.com>, Chris Mason <clm@fb.com>,
	Josef Bacik <jbacik@fb.com>, David Sterba <dsterba@suse.com>,
	linux-btrfs@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH] btrfs: fix dereference on inode->i_sb before inode null check
Date: Fri, 16 Dec 2016 15:24:41 +0000	[thread overview]
Message-ID: <2d2ef70f-c07d-8062-924d-e7d88b8d98fa@canonical.com> (raw)
In-Reply-To: <bdfa5c0f-758c-ad90-37b8-4d14516d5cdc@suse.com>


[-- Attachment #1.1: Type: text/plain, Size: 1892 bytes --]

On 16/12/16 15:03, Jeff Mahoney wrote:
> On 12/16/16 7:20 AM, Colin King wrote:
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> inode is being deferenced and then inode is checked to see if it
>> is null, implying we potentially could have a null pointer deference
>> on inode.
>>
>> Found with static analysis by CoverityScan, CID 1389472
>>
>> Fix this by dereferencing inode only after the inode null check.
>>
>> Fixes: 0b246afa62b0cf5 ("btrfs: root->fs_info cleanup, add fs_info convenience variables")
> 
> Hi Colin -
> 
> Thanks for the review.  The right fix here is to eliminate the tests for
> inode == NULL entirely.  This is a callback for exportfs, which will
> itself crash if dentry->d_inode or parent->d_inode is NULL.  Removing
> the tests would be consistent with other file systems.

Ah, thanks for pointing that out. New patch on it's way soon.

Colin

> 
> -Jeff
> 
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>> ---
>>  fs/btrfs/export.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/btrfs/export.c b/fs/btrfs/export.c
>> index 340d907..b746d2b 100644
>> --- a/fs/btrfs/export.c
>> +++ b/fs/btrfs/export.c
>> @@ -223,7 +223,7 @@ static int btrfs_get_name(struct dentry *parent, char *name,
>>  {
>>  	struct inode *inode = d_inode(child);
>>  	struct inode *dir = d_inode(parent);
>> -	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
>> +	struct btrfs_fs_info *fs_info;
>>  	struct btrfs_path *path;
>>  	struct btrfs_root *root = BTRFS_I(dir)->root;
>>  	struct btrfs_inode_ref *iref;
>> @@ -241,6 +241,7 @@ static int btrfs_get_name(struct dentry *parent, char *name,
>>  	if (!S_ISDIR(dir->i_mode))
>>  		return -EINVAL;
>>  
>> +	fs_info = btrfs_sb(inode->i_sb);
>>  	ino = btrfs_ino(inode);
>>  
>>  	path = btrfs_alloc_path();
>>
> 
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 837 bytes --]

      reply	other threads:[~2016-12-16 15:25 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-16 12:20 [PATCH] btrfs: fix dereference on inode->i_sb before inode null check Colin King
2016-12-16 15:03 ` Jeff Mahoney
2016-12-16 15:24   ` Colin Ian King [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=2d2ef70f-c07d-8062-924d-e7d88b8d98fa@canonical.com \
    --to=colin.king@canonical.com \
    --cc=clm@fb.com \
    --cc=dsterba@suse.com \
    --cc=jbacik@fb.com \
    --cc=jeffm@suse.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /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.