lkml.org 
[lkml]   [2019]   [Feb]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH][udf-next] udf: don't call mark_buffer_dirty on a null bh pointer
    From
    Date

    > On Feb 20, 2019, at 3:50 AM, Jan Kara <jack@suse.cz> wrote:
    >
    >> On Tue 19-02-19 08:17:09, Steve Magnani wrote:
    >>> On 2/19/19 8:02 AM, Jan Kara wrote:
    >>>> On Tue 19-02-19 11:44:03, Colin King wrote:
    >>>> From: Colin Ian King <colin.king@canonical.com>
    >>>>
    >>>> There is a null check on the pointer bh to avoid a null pointer dereference
    >>>> on bh->b_data however later bh is passed to mark_buffer_dirty that can also
    >>>> cause a null pointer dereference on bh. Avoid this potential null pointer
    >>>> dereference by moving the call to mark_buffer_dirty inside the null checked
    >>>> block.
    >>>>
    >>>> Fixes: e8b4274735e4 ("udf: finalize integrity descriptor before writeback")
    >>>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
    >>> Thanks for the patch! In fact it is the 'if (bh)' check that's
    >>> unnecessarily defensive (we cannot have sbi->s_lvid_dirty and
    >>> !sbi->s_lvid_bh). So I'll just drop that check (attached patch).
    >>>
    >>> Honza
    >>>
    >>>> ---
    >>>> fs/udf/super.c | 12 ++++++------
    >>>> 1 file changed, 6 insertions(+), 6 deletions(-)
    >>>>
    >>>> diff --git a/fs/udf/super.c b/fs/udf/super.c
    >>>> index a6940d90bedd..b7e9a83d39db 100644
    >>>> --- a/fs/udf/super.c
    >>>> +++ b/fs/udf/super.c
    >>>> @@ -2336,13 +2336,13 @@ static int udf_sync_fs(struct super_block *sb, int wait)
    >>>> lvid = (struct logicalVolIntegrityDesc *)bh->b_data;
    >>>> udf_finalize_lvid(lvid);
    >>>> - }
    >>>> - /*
    >>>> - * Blockdevice will be synced later so we don't have to submit
    >>>> - * the buffer for IO
    >>>> - */
    >>>> - mark_buffer_dirty(bh);
    >>>> + /*
    >>>> + * Blockdevice will be synced later so we don't have
    >>>> + * to submit the buffer for IO
    >>>> + */
    >>>> + mark_buffer_dirty(bh);
    >>>> + }
    >>>> sbi->s_lvid_dirty = 0;
    >>>> }
    >>>> mutex_unlock(&sbi->s_alloc_mutex);
    >>>> --
    >>>> 2.20.1
    >>>>
    >> Reviewed-by: Steven J. Magnani <steve@digidescorp.com>
    >
    > Is this Reviewed-by for my fixup or the Colin's? Because I've decided to
    > rather remove the 'if (bh)' check completely since it is pointless...
    >
    > Honza
    > --

    Sorry, I realized on rereading that this could be ambiguous. The R-B is for your patch.

    Steve
    \
     
     \ /
      Last update: 2019-02-20 12:28    [W:3.447 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site