commit c42dd069be8dfc9b2239a5c89e73bbd08ab35de0 upstream. Backporting the patch to stable-v5.10.y to avoid race condition between configfs_dir_lseek and configfs_lookup since they both operate ->s_childre and configfs_lookup forgets to obtain the lock. The patch deviates from the original patch because of code change. The idea is to hold the configfs_dirent_lock when traversing ->s_children, which follows the core idea of the original patch.
Signed-off-by: Kyle Zeng zengyhkyle@gmail.com --- fs/configfs/dir.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c index 12388ed4faa5..0b7e9ab517d5 100644 --- a/fs/configfs/dir.c +++ b/fs/configfs/dir.c @@ -479,6 +479,7 @@ static struct dentry * configfs_lookup(struct inode *dir, if (!configfs_dirent_is_ready(parent_sd)) goto out;
+ spin_lock(&configfs_dirent_lock); list_for_each_entry(sd, &parent_sd->s_children, s_sibling) { if (sd->s_type & CONFIGFS_NOT_PINNED) { const unsigned char * name = configfs_get_name(sd); @@ -491,6 +492,7 @@ static struct dentry * configfs_lookup(struct inode *dir, break; } } + spin_unlock(&configfs_dirent_lock);
if (!found) { /*
On Sat, Sep 02, 2023 at 01:20:36PM -0700, Kyle Zeng wrote:
commit c42dd069be8dfc9b2239a5c89e73bbd08ab35de0 upstream. Backporting the patch to stable-v5.10.y to avoid race condition between configfs_dir_lseek and configfs_lookup since they both operate ->s_childre and configfs_lookup forgets to obtain the lock. The patch deviates from the original patch because of code change. The idea is to hold the configfs_dirent_lock when traversing ->s_children, which follows the core idea of the original patch.
Signed-off-by: Kyle Zeng zengyhkyle@gmail.com
fs/configfs/dir.c | 2 ++ 1 file changed, 2 insertions(+)
You lost all the original signed-off-by lines of the original, AND you lost the authorship of the original commit. And you didn't cc: anyone involved in the original patch, to get their review, or objection to it being backported.
Take a look at many of the backports that happen on the stable list for examples of how to do this properly.
Here are 2 examples from this weekend alone that are good examples of how to do this properly: https://lore.kernel.org/r/20230902151000.3817-1-konishi.ryusuke@gmail.com https://lore.kernel.org/r/cover.1693593288.git.luizcap@amazon.com
Also, how did you test this change? is this something that you have actually hit in real life?
thanks,
greg k-h
You lost all the original signed-off-by lines of the original, AND you lost the authorship of the original commit. And you didn't cc: anyone involved in the original patch, to get their review, or objection to it being backported.
Sorry for the rookie mistakes. I drafted another version and it is attached to the email. Can you please check whether it is OK?
Also, how did you test this change? is this something that you have actually hit in real life?
Yes. I encountered this when testing the latest v5.10.y branch. A minimal proof-of-concept code looks like this: ~~~ #include <stdio.h> #include <stdlib.h> #include <fcntl.h> #include <unistd.h>
int main(void) { int fd = open("/sys/kernel/config", 0);
if(!fork()) { while(1) lseek(fd, SEEK_CUR, 1); } while(1) unlinkat(fd, "file", 0);
return 0; } ~~~
On Sat, Sep 02, 2023 at 02:55:14PM -0700, Kyle Zeng wrote:
You lost all the original signed-off-by lines of the original, AND you lost the authorship of the original commit. And you didn't cc: anyone involved in the original patch, to get their review, or objection to it being backported.
Sorry for the rookie mistakes. I drafted another version and it is attached to the email. Can you please check whether it is OK?
Good enough, thanks! Now queued up.
greg k-h
linux-stable-mirror@lists.linaro.org
-
Greg KH -
Kyle Zeng