[Bug 200951] New: kernel NULL pointer dereference in update_sit_entry

[Bug 200951] New: kernel NULL pointer dereference in update_sit_entry

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=200951

            Bug ID: 200951
           Summary: kernel NULL pointer dereference in update_sit_entry
           Product: File System
           Version: 2.5
    Kernel Version: v4.19-rc1, v4.18.2
          Hardware: ARM
                OS: Linux
              Tree: Mainline
            Status: NEW
          Severity: high
          Priority: P1
         Component: f2fs
          Assignee: filesystem_f2fs@kernel-bugs.kernel.org
          Reporter: vicencb@gmail.com
        Regression: No

Created attachment 278127
  --> https://bugzilla.kernel.org/attachment.cgi?id=278127&action=edit
dmesg

Hi,
in the setup there is a SATA SSD connected to a SATA-to-USB bridge.

The disc is "Samsung SSD 850 PRO 256G" which supports TRIM.
There are four partitions:
 sda1: FAT  /boot
 sda2: F2FS /
 sda3: F2FS /home
 sda4: F2FS

The bridge is ASMT1153e which uses the "uas" driver.
There is no TRIM pass-through, so, when mounting it reports:
 mounting with "discard" option, but the device does not support discard

The USB host is USB3.0 and UASP capable. It is the one on RK3399.

Given this everything works fine, except there is no TRIM support.

In order to enable TRIM a new UDEV rule is added [1]:
 /etc/udev/rules.d/10-sata-bridge-trim.rules:
 ACTION=="add|change", ATTRS{idVendor}=="174c", ATTRS{idProduct}=="55aa",
SUBSYSTEM=="scsi_disk", ATTR{provisioning_mode}="unmap"
After reboot any F2FS write hangs forever and dmesg reports:
 Unable to handle kernel NULL pointer dereference

Also tested on a x86_64 system: works fine even with TRIM enabled.
 same disc
 same bridge
 different usb host controller
 different cpu architecture
 not root filesystem

Regards,
  Vicenç.

[1] Post #5 in https://bbs.archlinux.org/viewtopic.php?id=236280

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[Bug 200951] kernel NULL pointer dereference in update_sit_entry

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=200951

--- Comment #1 from vicencb@gmail.com ---
Created attachment 278129
  --> https://bugzilla.kernel.org/attachment.cgi?id=278129&action=edit
sg_readcap

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

[Bug 200951] kernel NULL pointer dereference in update_sit_entry

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=200951

--- Comment #2 from vicencb@gmail.com ---
Created attachment 278131
  --> https://bugzilla.kernel.org/attachment.cgi?id=278131&action=edit
sg_vpd

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

[Bug 200951] kernel NULL pointer dereference in update_sit_entry

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=200951

Chao Yu (chao@kernel.org) changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |chao@kernel.org

--- Comment #3 from Chao Yu (chao@kernel.org) ---
Hi,

Is this bug reproducible? Can you run fsck on device to check filesystem
consistence.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

[Bug 200951] kernel NULL pointer dereference in update_sit_entry

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=200951

--- Comment #4 from vicencb@gmail.com ---
Hi! Yes, it is reproducible.
Each time TRIM is enabled via the UDEV rule and the FS mounted with
the discard option, the issue is triggered at the very first write attempt.
Before mounting fsck.f2fs was successfully run on the FS.

Regards,
  Vicenç.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[Bug 200951] kernel NULL pointer dereference in update_sit_entry

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=200951

--- Comment #5 from vicencb@gmail.com ---
Created attachment 278227
  --> https://bugzilla.kernel.org/attachment.cgi?id=278227&action=edit
fsck.f2fs

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

[Bug 200951] kernel NULL pointer dereference in update_sit_entry

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=200951

--- Comment #6 from vicencb@gmail.com ---
Hi, just found that the issue is also reproducible when mounted
with the "nodiscard" option, enabling TRIM via UDEV is enouth.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

[Bug 200951] kernel NULL pointer dereference in update_sit_entry

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=200951

--- Comment #7 from Chao Yu (chao@kernel.org) ---
Created attachment 278233
  --> https://bugzilla.kernel.org/attachment.cgi?id=278233&action=edit
add some log

Can you apply this patch attached and reproduce this bug again?

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

[Bug 200951] kernel NULL pointer dereference in update_sit_entry

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=200951

--- Comment #8 from vicencb@gmail.com ---
Created attachment 278241
  --> https://bugzilla.kernel.org/attachment.cgi?id=278241&action=edit
log results

Hi, here are the results asked for.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

[Bug 200951] kernel NULL pointer dereference in update_sit_entry

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=200951

--- Comment #9 from vicencb@gmail.com ---
Created attachment 278243
  --> https://bugzilla.kernel.org/attachment.cgi?id=278243&action=edit
add some log (bis)

Hi, added a few more printk for extra verbosity.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

[Bug 200951] kernel NULL pointer dereference in update_sit_entry

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=200951

--- Comment #10 from vicencb@gmail.com ---
Created attachment 278245
  --> https://bugzilla.kernel.org/attachment.cgi?id=278245&action=edit
log results (bis)

The results of the previous patch.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

[Bug 200951] kernel NULL pointer dereference in update_sit_entry

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=200951

--- Comment #11 from Chao Yu (chao@kernel.org) ---
Actually, the problem here is during mount(),  both blk_queue_discard(q) and
f2fs_sb_has_blkzoned(sbi) return false, so f2fs will skip allocating memory for
se->discard_map.

static inline bool f2fs_discard_en(struct f2fs_sb_info *sbi)
{
        struct request_queue *q = bdev_get_queue(sbi->sb->s_bdev);

        return blk_queue_discard(q) || f2fs_sb_has_blkzoned(sbi);
}

[    4.372580] f2fs_discard_en(1):0
[    4.498957] f2fs_discard_en(2):0

But later, during update_sit_entiry(), f2fs_discard_en() return true, then it
will cause f2fs to update se->discard_map bitmap, result in panic.

[   56.939547] f2fs_discard_en(3):1 new_blkaddr:0xFFFF00000AB0383C
[   56.945027] f2fs_discard_en(5):1 se:0xFFFF8000ED285ED8


Is there any interface we can turn on discard of device in real time?

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

[Bug 200951] kernel NULL pointer dereference in update_sit_entry

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=200951

--- Comment #12 from vicencb@gmail.com ---
As stated in the bug description, there is an udev rule to set
the provisioning_mode attribute to unmap. When searching for
this attribute name, i found this:
drivers/scsi/sd.c:749
  blk_queue_max_discard_sectors(q, ...);
  blk_queue_flag_set(QUEUE_FLAG_DISCARD, q);

So, it looks like the answer is yes, there is an interface to
turn on discard of device in real time.

Regards,
  Vicenç.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[Bug 200951] kernel NULL pointer dereference in update_sit_entry

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=200951

--- Comment #13 from Chao Yu (chao@kernel.org) ---
(In reply to vicencb from comment #12)
> As stated in the bug description, there is an udev rule to set

Sorry, I missed this one... :(

> the provisioning_mode attribute to unmap. When searching for
> this attribute name, i found this:
> drivers/scsi/sd.c:749
>   blk_queue_max_discard_sectors(q, ...);
>   blk_queue_flag_set(QUEUE_FLAG_DISCARD, q);
> 
> So, it looks like the answer is yes, there is an interface to
> turn on discard of device in real time.

Alright, let me send one patch to fix this issue.

> 
> Regards,
>   Vicenç.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[Bug 200951] kernel NULL pointer dereference in update_sit_entry

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=200951

--- Comment #14 from vicencb@gmail.com ---
This could explain why it fails on aarch64 but works on x86_64.
When this is tested on aarch64, the f2fs is the root FS, and the udev
rule is applied after mount because it is stored in the filesystemm itself.
Whereas in the x86_64 case, the udev rule is applied when the disk is
plugged in, before mounting.
All in all, it is a general issue, not an architecture-specific one.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

[Bug 200951] kernel NULL pointer dereference in update_sit_entry

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=200951

--- Comment #15 from Chao Yu (chao@kernel.org) ---
I've sent one patch for this issue, just cc your email, also you can find the
patch in below link[1], can you please try this patch to check whether it can
fix this issue?

[1] https://sourceforge.net/p/linux-f2fs/mailman/message/36407378/

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

[Bug 200951] kernel NULL pointer dereference in update_sit_entry

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=200951

--- Comment #16 from vicencb@gmail.com ---
Hi,
thank you for being that fast!

I have tried it against v4.19-rc2
 
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=57361846b52bc686112da6ca5368d11210796804
but it does not apply
  checking file fs/f2fs/debug.c
  Hunk #1 succeeded at 190 (offset -3 lines).
  checking file fs/f2fs/f2fs.h
  Hunk #1 FAILED at 3436.
  1 out of 1 hunk FAILED
  checking file fs/f2fs/file.c
  checking file fs/f2fs/segment.c
  Hunk #1 succeeded at 1725 (offset -43 lines).
  ...
  Hunk #8 succeeded at 3959 (offset -45 lines).
  checking file fs/f2fs/super.c
  Hunk #1 succeeded at 360 (offset -3 lines).
  Hunk #2 FAILED at 417.
  Hunk #3 succeeded at 1031 (offset -29 lines).
  Hunk #4 FAILED at 1440.
  2 out of 4 hunks FAILED

Which kernel tree should i be using?

Regards,
  Vicenç.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[Bug 200951] kernel NULL pointer dereference in update_sit_entry

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=200951

--- Comment #17 from Chao Yu (chao@kernel.org) ---
Updated in below git link, :)

https://git.kernel.org/pub/scm/linux/kernel/git/chao/linux.git/log/?h=f2fs-dev

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

[Bug 200951] kernel NULL pointer dereference in update_sit_entry

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=200951

Chao Yu (chao@kernel.org) changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |CODE_FIX

--- Comment #18 from Chao Yu (chao@kernel.org) ---
This issue has been fixed with patch listed in below link, so I tag this issue
as resolved one.

https://sourceforge.net/p/linux-f2fs/mailman/message/36407519/

-- 
You are receiving this mail because:
You are watching the assignee of the bug.