linux-kernel - Re: Patch for a overwriting/corruption of the file system

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, 14 Mar 2023 10:55:39 +0100
From:   Christian Brauner <brauner@...nel.org>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     Kirtikumar Anandrao Ramchandani <kirtiar15502@...il.com>,
        security@...nel.org, linux-kernel@...r.kernel.org,
        linux-fsdevel@...r.kernel.org,
        Linus Torvalds <torvalds@...uxfoundation.org>
Subject: Re: Patch for a overwriting/corruption of the file system

On Mon, Mar 13, 2023 at 11:32:08AM +0100, Greg KH wrote:
> On Mon, Mar 13, 2023 at 03:54:55PM +0530, Kirtikumar Anandrao Ramchandani wrote:
> > Seems like again it got rejected. I am sending it in the body if it works:
> > 
> > >From 839cae91705e044b49397590f2d85a5dd289f0c5 Mon Sep 17 00:00:00 2001
> > From: KirtiRamchandani <kirtar15502@...il.com>
> > Date: Mon, 13 Mar 2023 15:05:08 +0530
> > Subject: [PATCH] Fix bug in affs_rename() function. The `affs_rename()`
> >  function in the AFFS filesystem has a bug that can cause the `retval`
> >  variable to be overwritten before it is used. Specifically, the function
> >  assigns `retval` a value in two separate code blocks, but then only checks
> >  its value in one of those blocks. This commit fixes the bug by ensuring
> > that
> >  `retval` is properly checked in both code blocks.
> > 
> > Signed-off-by: KirtiRamchandani <kirtar15502@...il.com>
> > ---
> >  namei.c | 4++++--
> >  1 file changed, 4 insertions(+), 2 deletion(-)
> > 
> > diff --git a/fs/affs/namei.c b/fs/affs/namei.c
> > index d1084e5..a54c700 100644
> > --- a/fs/affs/namei.c
> > +++ b/fs/affs/namei.c
> > @@ -488,7 +488,8 @@ affs_xrename(struct inode *old_dir, struct dentry
> > *old_dentry,
> >         affs_lock_dir(new_dir);
> >         retval = affs_insert_hash(new_dir, bh_old);
> >         affs_unlock_dir(new_dir);
> > -
> > +       if (retval)
> > +               goto done;
> 
> The patch is corrupted and can not be applied.

Yeah, that patch is pretty borked. This should probably be sm like:

>From f3a7758bb53cc776820656c6ac66b13fb8ed9022 Mon Sep 17 00:00:00 2001
From: KirtiRamchandani <kirtar15502@...il.com>
Date: Tue, 14 Mar 2023 10:49:38 +0100
Subject: [PATCH] affs: handle errors in affs_xrename()

Fix a bug in the affs_xrename() function. The affs_xrename() function in
the AFFS filesystem has a bug that can cause the retval variable to be
overwritten before it is used. Specifically, the function assigns retval
a value in two separate code blocks, but then only checks its value in
one of those blocks. This commit fixes the bug by ensuring that retval
is properly checked in both code blocks.

Signed-off-by: KirtiRamchandani <kirtar15502@...il.com>
---
 fs/affs/namei.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/affs/namei.c b/fs/affs/namei.c
index d12ccfd2a83d..98525d69391d 100644
--- a/fs/affs/namei.c
+++ b/fs/affs/namei.c
@@ -488,6 +488,8 @@ affs_xrename(struct inode *old_dir, struct dentry *old_dentry,
 	affs_lock_dir(new_dir);
 	retval = affs_insert_hash(new_dir, bh_old);
 	affs_unlock_dir(new_dir);
+	if (retval)
+		goto done;
 
 	/* Insert new into the old directory with the old name. */
 	affs_copy_name(AFFS_TAIL(sb, bh_new)->name, old_dentry);
@@ -495,6 +497,8 @@ affs_xrename(struct inode *old_dir, struct dentry *old_dentry,
 	affs_lock_dir(old_dir);
 	retval = affs_insert_hash(old_dir, bh_new);
 	affs_unlock_dir(old_dir);
+	if (retval)
+		goto done;
 done:
 	mark_buffer_dirty_inode(bh_old, new_dir);
 	mark_buffer_dirty_inode(bh_new, old_dir);
-- 
2.34.1