linux-kernel - Re: [Ocfs2-devel] [PATCH] fs: ocfs2: move_extents.c: Fix for possible null pointer dereference

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] [day] [month] [year] [list]

Message-ID: <CAFo99gbv5x7tmLZqjygcvpR-tZ9bUAUEtocaGRCUaNPPPYq6PQ@mail.gmail.com>
Date:	Tue, 27 May 2014 22:21:19 +0200
From:	Rickard Strandqvist <rickard_strandqvist@...ctrumdigital.se>
To:	Jeff Liu <jeff.liu@...cle.com>
Cc:	Mark Fasheh <mfasheh@...e.com>, Joel Becker <jlbec@...lplan.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	ocfs2-devel@....oracle.com
Subject: Re: [Ocfs2-devel] [PATCH] fs: ocfs2: move_extents.c: Fix for possible
 null pointer dereference

Hi

Ok good, then everything is under control.
The result has been that I've done a number of these types of patches to :)

Sending a new patch of that kind then.


Best regards
Rickard Strandqvist


2014-05-26 5:28 GMT+02:00 Jeff Liu <jeff.liu@...cle.com>:
> Hi,
>
> On 05/23/2014 04:46 AM, Rickard Strandqvist wrote:
>> There is otherwise a risk of a possible null pointer dereference.
>>
>> Was largely found by using a static code analysis program called cppcheck.
>>
>> Signed-off-by: Rickard Strandqvist <rickard_strandqvist@...ctrumdigital.se>
>> ---
>>  fs/ocfs2/move_extents.c |    4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/ocfs2/move_extents.c b/fs/ocfs2/move_extents.c
>> index 599eb4c..6a8e3c8 100644
>> --- a/fs/ocfs2/move_extents.c
>> +++ b/fs/ocfs2/move_extents.c
>> @@ -902,11 +902,13 @@ static int ocfs2_move_extents(struct ocfs2_move_extents_context *context)
>>       struct inode *inode = context->inode;
>>       struct ocfs2_dinode *di;
>>       struct buffer_head *di_bh = NULL;
>> -     struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
>> +     struct ocfs2_super *osb;
>>
>>       if (!inode)
>>               return -ENOENT;
>>
>> +     osb = OCFS2_SB(inode->i_sb);
>> +
>>       if (ocfs2_is_hard_readonly(osb) || ocfs2_is_soft_readonly(osb))
>>               return -EROFS;
>
> Thanks for your patch, it looks reasonable if we consider it in the context
> of above function only. However, the inode should not be NULL in any case
> given that ocfs2_move_extents() is called by ocfs2_ioctl_move_extents() at
> where the inode is already validated.
>
> IMO, maybe we can just get rid of the useless inode pre-checkup, i.e,
>
> if (!inode)
>         return -ENOENT;
>
>
> Thanks,
> -Jeff
--
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/