Re: Bug: slab-out-of-bounds Write in __bh_read

Re: Bug: slab-out-of-bounds Write in __bh_read

[Jan Kara]

On Fri 10-01-25 17:21:19, Kun Hu wrote:
> > 
> > OK. Checking the syzlang reproducer the same comment as for other
> > filesystem bugs applies - please run with CONFIG_BLK_DEV_WRITE_MOUNTED
> > disabled to rule out corruption of a filesystem while it is mounted.
> > 
> > 
> 
> Hi Jan,
> 
> We have reproduced the crash with CONFIG_BLK_DEV_WRITE_MOUNTED disabled
> to obtain the same crash log. The new crash log, along with C and Syzlang
> reproducers are provided below:
> 
> Crash log: https://drive.google.com/file/d/1FiCgo05oPheAt4sDQzRYTQwl0-CY6rvi/view?usp=sharing
> C reproducer: https://drive.google.com/file/d/1TTR9cquaJcMYER6vtYUGh3gOn_mROME4/view?usp=sharing
> Syzlang reproducer: https://drive.google.com/file/d/1R9QDUP2r7MI4kYMiT_yn-tzm6NqmcEW-/view?usp=sharing
> 
> I’m not sure if this is sufficient to help locate the bug? If you need
> additional information, please let me know.

Thanks. Based on the crash report and the reproducer it indeed looks like
some mixing of iomap_folio_state and buffer heads attached to a folio
(iomap_folio_state is attached there but we end up calling
__block_write_begin_int() which expects buffer heads there) in GFS2. GFS2
guys, care to have a look?

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: Bug: slab-out-of-bounds Write in __bh_read

[Kun Hu]

> 
> Thanks. Based on the crash report and the reproducer it indeed looks like
> some mixing of iomap_folio_state and buffer heads attached to a folio
> (iomap_folio_state is attached there but we end up calling
> __block_write_begin_int() which expects buffer heads there) in GFS2. GFS2
> guys, care to have a look?
> 

Thanks to Jan.

Hi Andreas,

It seems that iomap_write_begin is expected to handle I/O directly based on folio, rather than entering the buffer head path. Is it possible that GFS2 incorrectly passes data related to buffer head to iomap_write_begin?

Could you please help us to check the exact cause of the issue?

Thanks,
Kun Hu

Re: Bug: slab-out-of-bounds Write in __bh_read

[Andreas Gruenbacher]

Hi Jan,

Am Fr., 10. Jan. 2025 um 18:18 Uhr schrieb Kun Hu <huk23@m.fudan.edu.cn>:
> > Thanks. Based on the crash report and the reproducer it indeed looks like
> > some mixing of iomap_folio_state and buffer heads attached to a folio
> > (iomap_folio_state is attached there but we end up calling
> > __block_write_begin_int() which expects buffer heads there) in GFS2. GFS2
> > guys, care to have a look?
> >
>
> Thanks to Jan.
>
> Hi Andreas,
>
> It seems that iomap_write_begin is expected to handle I/O directly based on folio, rather than entering the buffer head path. Is it possible that GFS2 incorrectly passes data related to buffer head to iomap_write_begin?
>
> Could you please help us to check the exact cause of the issue?

32generated_program.c memory maps the filesystem image, mounts it, and
then modifies it through the memory map. It's those modifications that
cause gfs2 to crash, so the test case is invalid.

Is disabling CONFIG_BLK_DEV_WRITE_MOUNTED supposed to prevent that? If
so, then it doesn't seem to be working.

Thanks,
Andreas

Re: Bug: slab-out-of-bounds Write in __bh_read

[Kun Hu]

> 
> 32generated_program.c memory maps the filesystem image, mounts it, and
> then modifies it through the memory map. It's those modifications that
> cause gfs2 to crash, so the test case is invalid.
> 
> Is disabling CONFIG_BLK_DEV_WRITE_MOUNTED supposed to prevent that? If
> so, then it doesn't seem to be working.
> 
> Thanks,
> Andreas


>  We have reproduced the crash with CONFIG_BLK_DEV_WRITE_MOUNTED disabled to obtain the same crash log. The new crash log, along with C and Syzlang reproducers are provided below:

> Crash log: https://drive.google.com/file/d/1FiCgo05oPheAt4sDQzRYTQwl0-CY6rvi/view?usp=sharing
> C reproducer: https://drive.google.com/file/d/1TTR9cquaJcMYER6vtYUGh3gOn_mROME4/view?usp=sharing
> Syzlang reproducer: https://drive.google.com/file/d/1R9QDUP2r7MI4kYMiT_yn-tzm6NqmcEW-/view?usp=sharing

Hi Andreas,

As per Jan's suggestion, we’ve successfully reproduced the crash with CONFIG_BLK_DEV_WRITE_MOUNTED disabled. Should you require us to test this issue again, we are happy to do so.

——
Thanks,
Kun Hu

Re: Bug: slab-out-of-bounds Write in __bh_read

[Andrew Price]

On 13/01/2025 15:54, Kun Hu wrote:
> 
>>
>> 32generated_program.c memory maps the filesystem image, mounts it, and
>> then modifies it through the memory map. It's those modifications that
>> cause gfs2 to crash, so the test case is invalid.
>>
>> Is disabling CONFIG_BLK_DEV_WRITE_MOUNTED supposed to prevent that? If
>> so, then it doesn't seem to be working.
>>
>> Thanks,
>> Andreas
> 
> 
>>   We have reproduced the crash with CONFIG_BLK_DEV_WRITE_MOUNTED disabled to obtain the same crash log. The new crash log, along with C and Syzlang reproducers are provided below:
> 
>> Crash log: https://drive.google.com/file/d/1FiCgo05oPheAt4sDQzRYTQwl0-CY6rvi/view?usp=sharing
>> C reproducer: https://drive.google.com/file/d/1TTR9cquaJcMYER6vtYUGh3gOn_mROME4/view?usp=sharing
>> Syzlang reproducer: https://drive.google.com/file/d/1R9QDUP2r7MI4kYMiT_yn-tzm6NqmcEW-/view?usp=sharing
> 
> Hi Andreas,
> 
> As per Jan's suggestion, we’ve successfully reproduced the crash with CONFIG_BLK_DEV_WRITE_MOUNTED disabled. Should you require us to test this issue again, we are happy to do so.
> 
FWIW the reproducer boils down to

   #include <fcntl.h>
   #include <unistd.h>
   #include <sys/ioctl.h>
   #include <linux/fs.h>

   /*
      mkfs.gfs2 -b 2048 -p lock_nolock $DEV
      mount $DEV $MNT
      cd $MNT
      /path/to/this_test
    */
   int main(void)
   {
           unsigned flag = FS_JOURNAL_DATA_FL;
           char buf[4102] = {0};
           int fd;

           /* Error checking omitted for clarity */
           fd = open("f", O_CREAT|O_RDWR);
           write(fd, buf, sizeof(buf));
           ioctl(fd, FS_IOC_SETFLAGS, &flag);
           write(fd, buf, sizeof(buf)); /* boom */
           close(fd);
           return 0;
   }

So it's switching the file to journaled data mode between two writes.

The size of the writes seems to be relevant and the fs needs to be 
created with a 2K block size (I'm guessing it could reproduce with other 
combinations).

Andy

Re: Bug: slab-out-of-bounds Write in __bh_read

[Andreas Gruenbacher]

On Mon, Jan 13, 2025 at 5:12 PM Andrew Price <anprice@redhat.com> wrote:
> On 13/01/2025 15:54, Kun Hu wrote:
> >
> >>
> >> 32generated_program.c memory maps the filesystem image, mounts it, and
> >> then modifies it through the memory map. It's those modifications that
> >> cause gfs2 to crash, so the test case is invalid.
> >>
> >> Is disabling CONFIG_BLK_DEV_WRITE_MOUNTED supposed to prevent that? If
> >> so, then it doesn't seem to be working.
> >>
> >> Thanks,
> >> Andreas
> >
> >
> >>   We have reproduced the crash with CONFIG_BLK_DEV_WRITE_MOUNTED disabled to obtain the same crash log. The new crash log, along with C and Syzlang reproducers are provided below:
> >
> >> Crash log: https://drive.google.com/file/d/1FiCgo05oPheAt4sDQzRYTQwl0-CY6rvi/view?usp=sharing
> >> C reproducer: https://drive.google.com/file/d/1TTR9cquaJcMYER6vtYUGh3gOn_mROME4/view?usp=sharing
> >> Syzlang reproducer: https://drive.google.com/file/d/1R9QDUP2r7MI4kYMiT_yn-tzm6NqmcEW-/view?usp=sharing
> >
> > Hi Andreas,
> >
> > As per Jan's suggestion, we’ve successfully reproduced the crash with CONFIG_BLK_DEV_WRITE_MOUNTED disabled. Should you require us to test this issue again, we are happy to do so.
> >
> FWIW the reproducer boils down to
>
>    #include <fcntl.h>
>    #include <unistd.h>
>    #include <sys/ioctl.h>
>    #include <linux/fs.h>
>
>    /*
>       mkfs.gfs2 -b 2048 -p lock_nolock $DEV
>       mount $DEV $MNT
>       cd $MNT
>       /path/to/this_test
>     */
>    int main(void)
>    {
>            unsigned flag = FS_JOURNAL_DATA_FL;
>            char buf[4102] = {0};
>            int fd;
>
>            /* Error checking omitted for clarity */
>            fd = open("f", O_CREAT|O_RDWR);
>            write(fd, buf, sizeof(buf));
>            ioctl(fd, FS_IOC_SETFLAGS, &flag);
>            write(fd, buf, sizeof(buf)); /* boom */
>            close(fd);
>            return 0;
>    }
>
> So it's switching the file to journaled data mode between two writes.
>
> The size of the writes seems to be relevant and the fs needs to be
> created with a 2K block size (I'm guessing it could reproduce with other
> combinations).

I've posted a fix and pushed it to for-next:

https://lore.kernel.org/gfs2/20250114175949.1196275-1-agruenba@redhat.com/

Thanks for reporting!

Andreas

Re: Bug: slab-out-of-bounds Write in __bh_read

[Andreas Gruenbacher]

On Wed, Jan 15, 2025 at 2:25 PM Kun Hu <huk23@m.fudan.edu.cn> wrote:
> I've posted a fix and pushed it to for-next:
> https://lore.kernel.org/gfs2/20250114175949.1196275-1-agruenba@redhat.com/
>
> Thanks for your help,
> Does the patch need us to test again?

That surely wouldn't hurt, thanks.

Andreas

Re: Bug: slab-out-of-bounds Write in __bh_read

[Kun Hu]

> 2025年1月15日 02:05，Andreas Gruenbacher <agruenba@redhat.com> 写道：
> 
> On Mon, Jan 13, 2025 at 5:12 PM Andrew Price <anprice@redhat.com> wrote:
>> On 13/01/2025 15:54, Kun Hu wrote:
>>> 
>>>> 
>>>> 32generated_program.c memory maps the filesystem image, mounts it, and
>>>> then modifies it through the memory map. It's those modifications that
>>>> cause gfs2 to crash, so the test case is invalid.
>>>> 
>>>> Is disabling CONFIG_BLK_DEV_WRITE_MOUNTED supposed to prevent that? If
>>>> so, then it doesn't seem to be working.
>>>> 
>>>> Thanks,
>>>> Andreas
>>> 
>>> 
>>>>  We have reproduced the crash with CONFIG_BLK_DEV_WRITE_MOUNTED disabled to obtain the same crash log. The new crash log, along with C and Syzlang reproducers are provided below:
>>> 
>>>> Crash log: https://drive.google.com/file/d/1FiCgo05oPheAt4sDQzRYTQwl0-CY6rvi/view?usp=sharing
>>>> C reproducer: https://drive.google.com/file/d/1TTR9cquaJcMYER6vtYUGh3gOn_mROME4/view?usp=sharing
>>>> Syzlang reproducer: https://drive.google.com/file/d/1R9QDUP2r7MI4kYMiT_yn-tzm6NqmcEW-/view?usp=sharing
>>> 
>>> Hi Andreas,
>>> 
>>> As per Jan's suggestion, we’ve successfully reproduced the crash with CONFIG_BLK_DEV_WRITE_MOUNTED disabled. Should you require us to test this issue again, we are happy to do so.
>>> 
>> FWIW the reproducer boils down to
>> 
>>   #include <fcntl.h>
>>   #include <unistd.h>
>>   #include <sys/ioctl.h>
>>   #include <linux/fs.h>
>> 
>>   /*
>>      mkfs.gfs2 -b 2048 -p lock_nolock $DEV
>>      mount $DEV $MNT
>>      cd $MNT
>>      /path/to/this_test
>>    */
>>   int main(void)
>>   {
>>           unsigned flag = FS_JOURNAL_DATA_FL;
>>           char buf[4102] = {0};
>>           int fd;
>> 
>>           /* Error checking omitted for clarity */
>>           fd = open("f", O_CREAT|O_RDWR);
>>           write(fd, buf, sizeof(buf));
>>           ioctl(fd, FS_IOC_SETFLAGS, &flag);
>>           write(fd, buf, sizeof(buf)); /* boom */
>>           close(fd);
>>           return 0;
>>   }
>> 
>> So it's switching the file to journaled data mode between two writes.
>> 
>> The size of the writes seems to be relevant and the fs needs to be
>> created with a 2K block size (I'm guessing it could reproduce with other
>> combinations).

Hi Andy,

Thanks for the reporting. I was unable to run the C reproducer you provided. I still reproduced the issue using syscall reproducer provided by syzkaller.

Thanks,

> 
> I've posted a fix and pushed it to for-next:
> 
> https://lore.kernel.org/gfs2/20250114175949.1196275-1-agruenba@redhat.com/
> 
> Thanks for reporting!
> 
> Andreas

> Syzlang reproducer: https://drive.google.com/file/d/1R9QDUP2r7MI4kYMiT_yn-tzm6NqmcEW-/view?usp=sharing

Hi Andreas,

Thank you for the patch. I tested it using the syscall reproducer and was still able to reproduce the issue.

Crash log: Link: https://github.com/pghk13/Kernel-Bug/blob/main/0103_6.13rc5_%E6%9C%AA%E6%8A%A5%E5%91%8A/%E5%AE%8C%E5%85%A8%E6%97%A0%E8%AE%B0%E5%BD%95/32-KASAN_%20slab-out-of-bounds%20Write%20in%20__bh_read/crashlog_0116_rc7%2Bpatch.txt

Could you confirm if the patch is intended to fully resolve this issue, or if additional changes might be required?

———
Thanks,
Kun Hu

Re: Bug: slab-out-of-bounds Write in __bh_read

[Andrew Price]

On 16/01/2025 10:41, Kun Hu wrote:
> 
>> 2025年1月15日 02:05，Andreas Gruenbacher <agruenba@redhat.com> 写道：
>> I've posted a fix and pushed it to for-next:
>>
>> https://lore.kernel.org/gfs2/20250114175949.1196275-1-agruenba@redhat.com/
>>
>> Thanks for reporting!
>>
>> Andreas
> 
>> Syzlang reproducer: https://drive.google.com/file/d/1R9QDUP2r7MI4kYMiT_yn-tzm6NqmcEW-/view?usp=sharing
> 
> Hi Andreas,
> 
> Thank you for the patch. I tested it using the syscall reproducer and was still able to reproduce the issue.
> 
> Crash log: Link: https://github.com/pghk13/Kernel-Bug/blob/main/0103_6.13rc5_%E6%9C%AA%E6%8A%A5%E5%91%8A/%E5%AE%8C%E5%85%A8%E6%97%A0%E8%AE%B0%E5%BD%95/32-KASAN_%20slab-out-of-bounds%20Write%20in%20__bh_read/crashlog_0116_rc7%2Bpatch.txt
> 

That's not the slab-out-of-bounds failure. It's failing to mount with:

"kobject: kobject_add_internal failed for syz:syz with -EEXIST, don't 
try to register things with the same name in the same directory."

This can happen when a gfs2 fs is mounted with the same locktable twice, 
so I expect it's a problem in the test. Please make sure you're running 
the same reproducer from the original report, in the same way. The patch 
fixes both reproducers for me.

Andy

Re: Bug: slab-out-of-bounds Write in __bh_read

[Andreas Gruenbacher]

On Mon, Jan 13, 2025 at 4:41 PM Andreas Gruenbacher <agruenba@redhat.com> wrote:
> Hi Jan,
>
> Am Fr., 10. Jan. 2025 um 18:18 Uhr schrieb Kun Hu <huk23@m.fudan.edu.cn>:
> > > Thanks. Based on the crash report and the reproducer it indeed looks like
> > > some mixing of iomap_folio_state and buffer heads attached to a folio
> > > (iomap_folio_state is attached there but we end up calling
> > > __block_write_begin_int() which expects buffer heads there) in GFS2. GFS2
> > > guys, care to have a look?
> > >
> >
> > Thanks to Jan.
> >
> > Hi Andreas,
> >
> > It seems that iomap_write_begin is expected to handle I/O directly based on folio, rather than entering the buffer head path. Is it possible that GFS2 incorrectly passes data related to buffer head to iomap_write_begin?
> >
> > Could you please help us to check the exact cause of the issue?
>
> 32generated_program.c memory maps the filesystem image, mounts it, and
> then modifies it through the memory map. It's those modifications that
> cause gfs2 to crash, so the test case is invalid.

Ah, I misread, the memory map is distinct from the filesystem. So
forget about disabling CONFIG_BLK_DEV_WRITE_MOUNTED not working.

Thanks,
Andreas