* Re: [PATCH] Fix: ext4: add sanity check for inode inline write range
2025-10-07 23:42 [PATCH] Fix: ext4: add sanity check for inode inline write range Ahmet Eray Karadag
@ 2025-10-08 0:47 ` Darrick J. Wong
2025-10-08 12:34 ` Theodore Ts'o
2025-10-10 11:31 ` kernel test robot
2 siblings, 0 replies; 6+ messages in thread
From: Darrick J. Wong @ 2025-10-08 0:47 UTC (permalink / raw)
To: Ahmet Eray Karadag
Cc: tytso, adilger.kernel, linux-ext4, linux-kernel,
david.hunter.linux, skhan, syzbot+f3185be57d7e8dda32b8,
Albin Babu Varghese
On Wed, Oct 08, 2025 at 02:42:22AM +0300, Ahmet Eray Karadag wrote:
> Add a simple check in ext4_try_to_write_inline_data() to prevent
> writes that extend past the inode's inline data area. The function
> now returns -EINVAL if pos + len exceeds i_inline_size.
>
> This avoids invalid inline write attempts and keeps the write path
> consistent with the inode limits.
>
> Reported-by: syzbot+f3185be57d7e8dda32b8@syzkaller.appspotmail.com
> Link: https://syzkaller.appspot.com/bug?extid=f3185be57d7e8dda32b8
> Co-developed-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
> Signed-off-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
> Signed-off-by: Ahmet Eray Karadag <eraykrdg1@gmail.com>
> ---
> fs/ext4/inline.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
> index 1b094a4f3866..13ba56e8e334 100644
> --- a/fs/ext4/inline.c
> +++ b/fs/ext4/inline.c
> @@ -782,6 +782,16 @@ int ext4_write_inline_data_end(struct inode *inode, loff_t pos, unsigned len,
> struct ext4_iloc iloc;
> int ret = 0, ret2;
>
> + if ((pos + len) > EXT4_I(inode)->i_inline_size) {
> + ext4_warning_inode(inode,
> + "inline write beyond capacity (pos=%lld, len=%u, inline_size=%d)",
> + pos, len, EXT4_I(inode)->i_inline_size);
> + folio_unlock(folio);
> + folio_put(folio);
> + ret = -EINVAL;
> + goto out;
Shouldn't write_begin have converted the file to block format if the
range to be written exceeds the possible inlinedata size?
> + }
> +
> if (unlikely(copied < len) && !folio_test_uptodate(folio))
> copied = 0;
>
> @@ -838,8 +848,8 @@ int ext4_write_inline_data_end(struct inode *inode, loff_t pos, unsigned len,
> */
> if (pos + len > inode->i_size && ext4_can_truncate(inode))
> ext4_orphan_add(handle, inode);
> -
> - ret2 = ext4_journal_stop(handle);
> + if (handle)
> + ret2 = ext4_journal_stop(handle);
What is this??
--D
> if (!ret)
> ret = ret2;
> if (pos + len > inode->i_size) {
> --
> 2.43.0
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Fix: ext4: add sanity check for inode inline write range
2025-10-07 23:42 [PATCH] Fix: ext4: add sanity check for inode inline write range Ahmet Eray Karadag
2025-10-08 0:47 ` Darrick J. Wong
@ 2025-10-08 12:34 ` Theodore Ts'o
2025-10-08 13:40 ` Ahmet Eray Karadag
2025-10-10 11:31 ` kernel test robot
2 siblings, 1 reply; 6+ messages in thread
From: Theodore Ts'o @ 2025-10-08 12:34 UTC (permalink / raw)
To: Ahmet Eray Karadag
Cc: adilger.kernel, linux-ext4, linux-kernel, david.hunter.linux,
skhan, syzbot+f3185be57d7e8dda32b8, Albin Babu Varghese
On Wed, Oct 08, 2025 at 02:42:22AM +0300, Ahmet Eray Karadag wrote:
> Add a simple check in ext4_try_to_write_inline_data() to prevent
> writes that extend past the inode's inline data area. The function
> now returns -EINVAL if pos + len exceeds i_inline_size.
The commit description doesn't match with what the patch does. The
patch changes ext4_write_inline_data_end() and not
ext4_try_to_write_inline(). Ext4_try_to_write_inline_data() called
from ext4_write_begin(), and it does this:
if (pos + len > ext4_get_max_inline_size(inode))
return ext4_convert_inline_data_to_extent(mapping, inode);
So the write extends past the inline data area, in ext4_write_begin(),
it will have already been converted to a non-inline function.
The ext4_write_inline_data_end() function is called from
ext4_write_end(), so you need to figure out why we hadn't configured
the file away from inline data in ext4_write_begin().
> Reported-by: syzbot+f3185be57d7e8dda32b8@syzkaller.appspotmail.com
> Link: https://syzkaller.appspot.com/bug?extid=f3185be57d7e8dda32b8
Did you just randomly bash the code until the syzbot reproducer
stopped failing? Please try to understand the code and the failure
much more deeply before attempting to change the code.
Cheers,
- Ted
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix: ext4: add sanity check for inode inline write range
2025-10-08 12:34 ` Theodore Ts'o
@ 2025-10-08 13:40 ` Ahmet Eray Karadag
2025-10-09 14:21 ` David Hunter
0 siblings, 1 reply; 6+ messages in thread
From: Ahmet Eray Karadag @ 2025-10-08 13:40 UTC (permalink / raw)
To: Theodore Ts'o
Cc: adilger.kernel, linux-ext4, linux-kernel, david.hunter.linux,
skhan, syzbot+f3185be57d7e8dda32b8, Albin Babu Varghese
Hi Ted,
Thanks for the feedback.
You’re right — the commit description didn’t match the actual change,
and I appreciate you pointing that out.
After reading Darrick’s reply, I realized our patch was only
preventing the final crash rather than fixing the underlying issue.
Sorry for the confusion — I noticed that a bit too late after sending it out.
I’ll review things more carefully before sending any follow-ups.
Thanks again for the guidance.
Best regards,
Eray
Theodore Ts'o <tytso@mit.edu>, 8 Eki 2025 Çar, 15:34 tarihinde şunu yazdı:
>
> On Wed, Oct 08, 2025 at 02:42:22AM +0300, Ahmet Eray Karadag wrote:
> > Add a simple check in ext4_try_to_write_inline_data() to prevent
> > writes that extend past the inode's inline data area. The function
> > now returns -EINVAL if pos + len exceeds i_inline_size.
>
> The commit description doesn't match with what the patch does. The
> patch changes ext4_write_inline_data_end() and not
> ext4_try_to_write_inline(). Ext4_try_to_write_inline_data() called
> from ext4_write_begin(), and it does this:
>
> if (pos + len > ext4_get_max_inline_size(inode))
> return ext4_convert_inline_data_to_extent(mapping, inode);
>
> So the write extends past the inline data area, in ext4_write_begin(),
> it will have already been converted to a non-inline function.
>
> The ext4_write_inline_data_end() function is called from
> ext4_write_end(), so you need to figure out why we hadn't configured
> the file away from inline data in ext4_write_begin().
>
> > Reported-by: syzbot+f3185be57d7e8dda32b8@syzkaller.appspotmail.com
> > Link: https://syzkaller.appspot.com/bug?extid=f3185be57d7e8dda32b8
>
> Did you just randomly bash the code until the syzbot reproducer
> stopped failing? Please try to understand the code and the failure
> much more deeply before attempting to change the code.
>
> Cheers,
>
> - Ted
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix: ext4: add sanity check for inode inline write range
2025-10-08 13:40 ` Ahmet Eray Karadag
@ 2025-10-09 14:21 ` David Hunter
0 siblings, 0 replies; 6+ messages in thread
From: David Hunter @ 2025-10-09 14:21 UTC (permalink / raw)
To: Ahmet Eray Karadag, Theodore Ts'o
Cc: adilger.kernel, linux-ext4, linux-kernel, skhan,
syzbot+f3185be57d7e8dda32b8, Albin Babu Varghese
On 10/8/25 09:40, Ahmet Eray Karadag wrote:
> I’ll review things more carefully before sending any follow-ups.
Hey,
I wanted to make some suggestions for you to do before sending a patch.
I recommend using tools like ftrace and the old classic printf
statements to verify that the code is doing what you are saying that the
code is doing.
If you are having trouble with the less intrusive tools, you can use
kgdb to verify your own code does what is expected, but do not use it as
a final verification because it is too intrusive and changes overall
kernel behavior.
Let me know if you are having any problems or need help with something.
Thanks,
David Hunter
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix: ext4: add sanity check for inode inline write range
2025-10-07 23:42 [PATCH] Fix: ext4: add sanity check for inode inline write range Ahmet Eray Karadag
2025-10-08 0:47 ` Darrick J. Wong
2025-10-08 12:34 ` Theodore Ts'o
@ 2025-10-10 11:31 ` kernel test robot
2 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2025-10-10 11:31 UTC (permalink / raw)
To: Ahmet Eray Karadag, tytso, adilger.kernel
Cc: oe-kbuild-all, linux-ext4, linux-kernel, david.hunter.linux,
skhan, Ahmet Eray Karadag, syzbot+f3185be57d7e8dda32b8,
Albin Babu Varghese
Hi Ahmet,
kernel test robot noticed the following build warnings:
[auto build test WARNING on tytso-ext4/dev]
[also build test WARNING on linus/master v6.17 next-20251009]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Ahmet-Eray-Karadag/Fix-ext4-add-sanity-check-for-inode-inline-write-range/20251010-013008
base: https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
patch link: https://lore.kernel.org/r/20251007234221.28643-2-eraykrdg1%40gmail.com
patch subject: [PATCH] Fix: ext4: add sanity check for inode inline write range
config: powerpc-randconfig-r073-20251010 (https://download.01.org/0day-ci/archive/20251010/202510101841.kobch6UW-lkp@intel.com/config)
compiler: powerpc-linux-gcc (GCC) 14.3.0
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510101841.kobch6UW-lkp@intel.com/
smatch warnings:
fs/ext4/inline.c:789 ext4_write_inline_data_end() warn: inconsistent indenting
fs/ext4/inline.c:854 ext4_write_inline_data_end() error: uninitialized symbol 'ret2'.
vim +789 fs/ext4/inline.c
775
776 int ext4_write_inline_data_end(struct inode *inode, loff_t pos, unsigned len,
777 unsigned copied, struct folio *folio)
778 {
779 handle_t *handle = ext4_journal_current_handle();
780 int no_expand;
781 void *kaddr;
782 struct ext4_iloc iloc;
783 int ret = 0, ret2;
784
785 if ((pos + len) > EXT4_I(inode)->i_inline_size) {
786 ext4_warning_inode(inode,
787 "inline write beyond capacity (pos=%lld, len=%u, inline_size=%d)",
788 pos, len, EXT4_I(inode)->i_inline_size);
> 789 folio_unlock(folio);
790 folio_put(folio);
791 ret = -EINVAL;
792 goto out;
793 }
794
795 if (unlikely(copied < len) && !folio_test_uptodate(folio))
796 copied = 0;
797
798 if (likely(copied)) {
799 ret = ext4_get_inode_loc(inode, &iloc);
800 if (ret) {
801 folio_unlock(folio);
802 folio_put(folio);
803 ext4_std_error(inode->i_sb, ret);
804 goto out;
805 }
806 ext4_write_lock_xattr(inode, &no_expand);
807 BUG_ON(!ext4_has_inline_data(inode));
808
809 /*
810 * ei->i_inline_off may have changed since
811 * ext4_write_begin() called
812 * ext4_try_to_write_inline_data()
813 */
814 (void) ext4_find_inline_data_nolock(inode);
815
816 kaddr = kmap_local_folio(folio, 0);
817 ext4_write_inline_data(inode, &iloc, kaddr, pos, copied);
818 kunmap_local(kaddr);
819 folio_mark_uptodate(folio);
820 /* clear dirty flag so that writepages wouldn't work for us. */
821 folio_clear_dirty(folio);
822
823 ext4_write_unlock_xattr(inode, &no_expand);
824 brelse(iloc.bh);
825
826 /*
827 * It's important to update i_size while still holding folio
828 * lock: page writeout could otherwise come in and zero
829 * beyond i_size.
830 */
831 ext4_update_inode_size(inode, pos + copied);
832 }
833 folio_unlock(folio);
834 folio_put(folio);
835
836 /*
837 * Don't mark the inode dirty under folio lock. First, it unnecessarily
838 * makes the holding time of folio lock longer. Second, it forces lock
839 * ordering of folio lock and transaction start for journaling
840 * filesystems.
841 */
842 if (likely(copied))
843 mark_inode_dirty(inode);
844 out:
845 /*
846 * If we didn't copy as much data as expected, we need to trim back
847 * size of xattr containing inline data.
848 */
849 if (pos + len > inode->i_size && ext4_can_truncate(inode))
850 ext4_orphan_add(handle, inode);
851 if (handle)
852 ret2 = ext4_journal_stop(handle);
853 if (!ret)
> 854 ret = ret2;
855 if (pos + len > inode->i_size) {
856 ext4_truncate_failed_write(inode);
857 /*
858 * If truncate failed early the inode might still be
859 * on the orphan list; we need to make sure the inode
860 * is removed from the orphan list in that case.
861 */
862 if (inode->i_nlink)
863 ext4_orphan_del(NULL, inode);
864 }
865 return ret ? ret : copied;
866 }
867
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 6+ messages in thread