On Tue, Mar 14, 2023 at 10:55:39AM +0100, Christian Brauner wrote: > 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@xxxxxxxxx> > > > 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@xxxxxxxxx> > > > --- > > > 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@xxxxxxxxx> > 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. "Properly checked" as in...? > Signed-off-by: KirtiRamchandani <kirtar15502@xxxxxxxxx> > --- > 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; OK, so you've got an IO error and insertion has failed. Both entries had already been removed from their directories. Sure, we must report an error, but why is leaking *both* entries the right thing to do? > /* 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: Really? How could that possibly make any sense? I mean, look for the target of that goto... The bug here (AFFS awful layout aside) is that error from the first insert_hash is always lost. And it needs to be reported. But this is no way to fix that.
