linux-ext4 - Re: [PATCH 1/2] jbd2: check bh->b_data for NULL in jbd2_journal_get_descriptor

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]

Date:	Tue, 4 Jun 2013 14:15:57 +0300
From:	Ruslan Bilovol <ruslan.bilovol@...com>
To:	"Theodore Ts'o" <tytso@....edu>,
	Ruslan Bilovol <ruslan.bilovol@...com>,
	adilger.kernel@...ger.ca, linux-ext4@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] jbd2: check bh->b_data for NULL in
 jbd2_journal_get_descriptor_buffer before memset()

Hi Ted,

On Mon, Jun 3, 2013 at 6:33 PM, Theodore Ts'o <tytso@....edu> wrote:
> On Mon, Jun 03, 2013 at 01:00:15PM +0300, Ruslan Bilovol wrote:
>> The memset() doesn't perform any NULL-pointer checking
>> before dereferencing passed pointer so this should be
>> checked before calling it.
>
> I can see that __getblk() can return NULL if there is a memory
> allocation failure (and is defined to do so), so checking to make sure
> bh is not NULL is a good thing to do.
>
> Have you actually seen a case where bh is non-NULL, but bh->b_data is
> NULL?  If not, it might be better to do something like this:

Yes, this is exactly the situation I observe (bh is non-NULL, but
bh->b_data is NULL)

>
>>       bh = __getblk(journal->j_dev, blocknr, journal->j_blocksize);
>         if (!bh)
>                 return NULL;
>         BUG_ON(!bh->b_data);

Is it so critical that we need to stop the kernel here?
Can we recover from this state gracefully?
Maybe something like this may be better:

       bh = __getblk(journal->j_dev, blocknr, journal->j_blocksize);
         if (!bh)
                 return NULL;
         if(!bh->b_data) {
                 WARN_ON(1);
                 return NULL;
         }

Regards,
Ruslan

>
>                                                 - Ted
> --
> 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/



-- 
Best regards,
Ruslan Bilvol
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html