Skip to content

fs: Enhance read-only filesystem error notification mechanism #1268

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
electricface wants to merge 1 commit into
base: linux-6.6.y
Choose a base branch
from
Open

fs: Enhance read-only filesystem error notification mechanism #1268

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
696 changes: 582 additions & 114 deletions fs/deepin_err_notify.c
View file
Open in desktop

Large diffs are not rendered by default.

27 changes: 24 additions & 3 deletions fs/internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,35 @@ int do_mkdirat(int dfd, struct filename *name, umode_t mode);
int do_symlinkat(struct filename *from, int newdfd, struct filename *to);
int do_linkat(int olddfd, struct filename *old, int newdfd,
struct filename *new, int flags);
int deepin_get_path_for_err_notify(int dfd, struct filename *name, struct path *result_path);
struct deepin_path_last {
struct path path;
const char *last;
};

int deepin_lookup_path_or_parent(int dfd, struct filename *name,
unsigned int flags,
struct deepin_path_last *result_path_last);

/*
* deepin_err_notify.c
*/
int deepin_err_notify_enabled(void);
void deepin_check_and_notify_ro_fs_err(const struct path *path, const char *func_name);
void deepin_send_ro_fs_err_notification(const char *filename, const char *func_name);
int deepin_err_notify_should_send(void);
void deepin_check_and_notify_ro_fs_err(const struct deepin_path_last *path_last,
const char *func_name);
void deepin_notify_rename_ro_fs_err(const struct qstr *old_last,
const struct qstr *new_last,
const struct path *old_path,
const struct path *new_path);
void deepin_put_path_last(struct deepin_path_last *path_last);

#ifdef CONFIG_DEEPIN_ERR_NOTIFY
/* Check if error is EROFS and notification should be sent */
#define deepin_should_notify_ro_fs_err(error) \
unlikely((error) == -EROFS && deepin_err_notify_should_send())
#else
#define deepin_should_notify_ro_fs_err(error) 0
#endif /* CONFIG_DEEPIN_ERR_NOTIFY */

/*
* namespace.c
Expand Down
11 changes: 7 additions & 4 deletions fs/ioctl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -903,10 +903,13 @@ SYSCALL_DEFINE3(ioctl, unsigned int, fd, unsigned int, cmd, unsigned long, arg)
if (error == -ENOIOCTLCMD)
error = vfs_ioctl(f.file, cmd, arg);

#ifdef CONFIG_DEEPIN_ERR_NOTIFY
if (unlikely((error == -EROFS) && deepin_err_notify_enabled()))
deepin_check_and_notify_ro_fs_err(&f.file->f_path, "ioctl");
#endif /* CONFIG_DEEPIN_ERR_NOTIFY */
if (deepin_should_notify_ro_fs_err(error)) {
struct deepin_path_last tmp_path_last = {
.path = f.file->f_path,
.last = NULL
};
deepin_check_and_notify_ro_fs_err(&tmp_path_last, "ioctl");
}

out:
fdput(f);
Expand Down
203 changes: 101 additions & 102 deletions fs/namei.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4073,19 +4073,14 @@ static int do_mknodat(int dfd, struct filename *name, umode_t mode,
goto retry;
}
out1:
#ifdef CONFIG_DEEPIN_ERR_NOTIFY
if (unlikely((error == -EROFS) && deepin_err_notify_enabled())) {
struct path file_path;
int get_path_err;

get_path_err =
deepin_get_path_for_err_notify(dfd, name, &file_path);
if (!get_path_err) {
deepin_check_and_notify_ro_fs_err(&file_path, "mknod");
path_put(&file_path);
if (deepin_should_notify_ro_fs_err(error)) {
struct deepin_path_last path_last;

if (!deepin_lookup_path_or_parent(dfd, name, lookup_flags, &path_last)) {
deepin_check_and_notify_ro_fs_err(&path_last, "mknod");
deepin_put_path_last(&path_last);
Comment on lines +4079 to +4081
Copy link

@sourcery-ai sourcery-ai bot Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): deepin_lookup_path_or_parent treats qstr.name as a C-string, but qstr names are not guaranteed to be NUL-terminated.

In deepin_lookup_path_or_parent() you do:

result_path_last->last = kstrdup((const char *)last.name, GFP_KERNEL);

and later combine_path_and_last() calls strlen(last) on that buffer. Since struct qstr only guarantees name + len, this can read past the allocated memory and lead to corruption.

Use:

  • kstrndup(last.name, last.len, GFP_KERNEL) when duplicating, and
  • propagate last.len (or an equivalent length) instead of calling strlen(last).

Apply the same pattern anywhere last.name is handled as a C string.

}
}
#endif /* CONFIG_DEEPIN_ERR_NOTIFY */
putname(name);
return error;
}
Expand Down Expand Up @@ -4169,19 +4164,14 @@ int do_mkdirat(int dfd, struct filename *name, umode_t mode)
goto retry;
}
out_putname:
#ifdef CONFIG_DEEPIN_ERR_NOTIFY
if (unlikely((error == -EROFS) && deepin_err_notify_enabled())) {
struct path file_path;
int get_path_err;

get_path_err =
deepin_get_path_for_err_notify(dfd, name, &file_path);
if (!get_path_err) {
deepin_check_and_notify_ro_fs_err(&file_path, "mkdir");
path_put(&file_path);
if (deepin_should_notify_ro_fs_err(error)) {
struct deepin_path_last path_last;

if (!deepin_lookup_path_or_parent(dfd, name, lookup_flags, &path_last)) {
deepin_check_and_notify_ro_fs_err(&path_last, "mkdir");
deepin_put_path_last(&path_last);
}
}
#endif /* CONFIG_DEEPIN_ERR_NOTIFY */
putname(name);
return error;
}
Expand Down Expand Up @@ -4299,21 +4289,18 @@ int do_rmdir(int dfd, struct filename *name)
inode_unlock(path.dentry->d_inode);
mnt_drop_write(path.mnt);
exit2:
#ifdef CONFIG_DEEPIN_ERR_NOTIFY
if (unlikely((error == -EROFS) && deepin_err_notify_enabled())) {
dentry = lookup_one_qstr_excl(&last, path.dentry, 0);
if (!IS_ERR(dentry)) {
if (d_is_positive(dentry)) {
// dentry is positive, so we can get the path
struct path file_path = { .mnt = path.mnt,
.dentry = dentry };
deepin_check_and_notify_ro_fs_err(&file_path,
if (deepin_should_notify_ro_fs_err(error)) {
struct deepin_path_last path_last;

if (!deepin_lookup_path_or_parent(dfd, name, lookup_flags, &path_last)) {
if (!path_last.last) {
// File exists, notify error
deepin_check_and_notify_ro_fs_err(&path_last,
"rmdir");
}
dput(dentry);
deepin_put_path_last(&path_last);
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Memory leak when path lookup fails in the middle. When deepin_lookup_path_or_parent succeeds but the subsequent check at line 4296 (if (!path_last.last)) is false, the function returns without calling deepin_put_path_last(&path_last). This means the path reference acquired in deepin_lookup_path_or_parent is never released when path_last.last is not NULL. The cleanup call should be moved outside the conditional block or a call should be added to the else branch.

Suggested change
deepin_put_path_last(&path_last);
deepin_put_path_last(&path_last);
} else {
deepin_put_path_last(&path_last);

Copilot uses AI. Check for mistakes.
}
}
#endif /* CONFIG_DEEPIN_ERR_NOTIFY */
path_put(&path);
if (retry_estale(error, lookup_flags)) {
lookup_flags |= LOOKUP_REVAL;
Expand Down Expand Up @@ -4459,21 +4446,18 @@ int do_unlinkat(int dfd, struct filename *name)
}
mnt_drop_write(path.mnt);
exit2:
#ifdef CONFIG_DEEPIN_ERR_NOTIFY
if (unlikely((error == -EROFS) && deepin_err_notify_enabled())) {
dentry = lookup_one_qstr_excl(&last, path.dentry, 0);
if (!IS_ERR(dentry)) {
if (d_is_positive(dentry)) {
// dentry is positive, so we can get the path
struct path file_path = { .mnt = path.mnt,
.dentry = dentry };
deepin_check_and_notify_ro_fs_err(&file_path,
if (deepin_should_notify_ro_fs_err(error)) {
struct deepin_path_last path_last;

if (!deepin_lookup_path_or_parent(dfd, name, lookup_flags, &path_last)) {
if (!path_last.last) {
// File exists, notify error
deepin_check_and_notify_ro_fs_err(&path_last,
"unlink");
}
dput(dentry);
deepin_put_path_last(&path_last);
}
Comment on lines +4449 to 4459
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Memory leak when path lookup fails in the middle. When deepin_lookup_path_or_parent succeeds but the subsequent check at line 4453 (if (!path_last.last)) is false, the function returns without calling deepin_put_path_last(&path_last). This means the path reference acquired in deepin_lookup_path_or_parent is never released when path_last.last is not NULL. The cleanup call should be moved outside the conditional block or a call should be added to the else branch.

Copilot uses AI. Check for mistakes.
}
#endif /* CONFIG_DEEPIN_ERR_NOTIFY */
path_put(&path);
if (retry_estale(error, lookup_flags)) {
lookup_flags |= LOOKUP_REVAL;
Expand Down Expand Up @@ -4574,20 +4558,15 @@ int do_symlinkat(struct filename *from, int newdfd, struct filename *to)
goto retry;
}
out_putnames:
#ifdef CONFIG_DEEPIN_ERR_NOTIFY
if (unlikely((error == -EROFS) && deepin_err_notify_enabled())) {
struct path file_path;
int get_path_err;

get_path_err =
deepin_get_path_for_err_notify(newdfd, to, &file_path);
if (!get_path_err) {
deepin_check_and_notify_ro_fs_err(&file_path,
if (deepin_should_notify_ro_fs_err(error)) {
struct deepin_path_last path_last;

if (!deepin_lookup_path_or_parent(newdfd, to, lookup_flags, &path_last)) {
deepin_check_and_notify_ro_fs_err(&path_last,
"symlink");
path_put(&file_path);
deepin_put_path_last(&path_last);
}
}
#endif /* CONFIG_DEEPIN_ERR_NOTIFY */
putname(to);
putname(from);
return error;
Expand Down Expand Up @@ -4766,19 +4745,14 @@ int do_linkat(int olddfd, struct filename *old, int newdfd,
goto retry;
}
out_putpath:
#ifdef CONFIG_DEEPIN_ERR_NOTIFY
if (unlikely((error == -EROFS) && deepin_err_notify_enabled())) {
struct path file_path;
int get_path_err;

get_path_err =
deepin_get_path_for_err_notify(newdfd, new, &file_path);
if (!get_path_err) {
deepin_check_and_notify_ro_fs_err(&file_path, "link");
path_put(&file_path);
if (deepin_should_notify_ro_fs_err(error)) {
struct deepin_path_last path_last;

if (!deepin_lookup_path_or_parent(newdfd, new, how, &path_last)) {
deepin_check_and_notify_ro_fs_err(&path_last, "link");
deepin_put_path_last(&path_last);
}
}
#endif /* CONFIG_DEEPIN_ERR_NOTIFY */
path_put(&old_path);
out_putnames:
putname(old);
Expand Down Expand Up @@ -5133,22 +5107,8 @@ int do_renameat2(int olddfd, struct filename *from, int newdfd,
}
mnt_drop_write(old_path.mnt);
exit2:
#ifdef CONFIG_DEEPIN_ERR_NOTIFY
if (unlikely((error == -EROFS) && deepin_err_notify_enabled())) {
old_dentry =
lookup_one_qstr_excl(&old_last, old_path.dentry, 0);
if (!IS_ERR(old_dentry)) {
if (d_is_positive(old_dentry)) {
struct path file_path = { .mnt = old_path.mnt,
.dentry =
old_dentry };
deepin_check_and_notify_ro_fs_err(&file_path,
"rename");
}
dput(old_dentry);
}
}
#endif /* CONFIG_DEEPIN_ERR_NOTIFY */
if (deepin_should_notify_ro_fs_err(error))
deepin_notify_rename_ro_fs_err(&old_last, &new_last, &old_path, &new_path);
Comment on lines +5110 to +5111
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function deepin_notify_rename_ro_fs_err is called at line 5111 to handle rename errors, but it directly uses old_last->name and new_last->name pointers from qstr structures (see lines 671-672 in deepin_err_notify.c). These pointers are stack-allocated or reference dentry names that may not remain valid. Unlike other call sites that use deepin_lookup_path_or_parent which duplicates strings with kstrdup() (line 5381 in namei.c), this rename handler doesn't duplicate the strings, potentially causing use-after-free issues.

Suggested change
if (deepin_should_notify_ro_fs_err(error))
deepin_notify_rename_ro_fs_err(&old_last, &new_last, &old_path, &new_path);
if (deepin_should_notify_ro_fs_err(error)) {
struct qstr old_last_dup = old_last;
struct qstr new_last_dup = new_last;
old_last_dup.name = kstrdup(old_last.name, GFP_KERNEL);
new_last_dup.name = kstrdup(new_last.name, GFP_KERNEL);
deepin_notify_rename_ro_fs_err(&old_last_dup, &new_last_dup, &old_path, &new_path);
kfree(old_last_dup.name);
kfree(new_last_dup.name);
}

Copilot uses AI. Check for mistakes.
if (retry_estale(error, lookup_flags))
should_retry = true;
path_put(&new_path);
Expand Down Expand Up @@ -5368,37 +5328,76 @@ const struct inode_operations page_symlink_inode_operations = {
EXPORT_SYMBOL(page_symlink_inode_operations);

#ifdef CONFIG_DEEPIN_ERR_NOTIFY
int deepin_get_path_for_err_notify(int dfd, struct filename *name,
struct path *result_path)
/**
* deepin_lookup_path_or_parent - Prepare path info for read-only FS error notification
* @dfd: Directory file descriptor used as lookup base
* @name: Filename to look up (struct filename)
* @flags: Lookup flags (e.g., LOOKUP_DIRECTORY, LOOKUP_FOLLOW)
* @result_path_last: Output structure carrying the resolved parent path and
* optionally the last component string
*
* Memory/Lifetime management:
* - result_path_last->path:
* The path returned by filename_lookup() or filename_parentat() already
* holds a reference count. You MUST release it after use. Preferred:
* deepin_put_path_last(&pl), which will call path_put() for you.
* Alternatively, call path_put() manually if you manage the string separately.
*
* - result_path_last->last:
* In the -ENOENT (parent found) case, the last component string is duplicated
* via kstrdup() and MUST be freed with kfree(). deepin_put_path_last(&pl)
* will free it for you. If the full path was resolved, this field is set
* to NULL and no extra string free is needed.
*
* Recommended usage pattern:
* struct deepin_path_last pl;
* if (!deepin_lookup_path_or_parent(dfd, name, lookup_flags, &pl)) {
* deepin_check_and_notify_ro_fs_err(&pl, "op");
* deepin_put_path_last(&pl); // releases path and frees pl.last if allocated
* }
*
* Return: 0 on success, negative errno on failure.
*/
int deepin_lookup_path_or_parent(int dfd, struct filename *name,
unsigned int flags,
struct deepin_path_last *result_path_last)
{
struct path result_path;
struct path parent;
struct qstr last;
struct path parent_path;
int type;
struct dentry *dentry;
int error;

error = filename_parentat(dfd, name, 0, &parent_path, &last, &type);
error = filename_lookup(dfd, name, flags, &result_path, NULL);
if (error == -ENOENT) {
error = filename_parentat(dfd, name, flags, &parent, &last, &type);
if (error)
Copy link

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] When handling the -ENOENT case, if filename_parentat() returns an error at line 5374, the function returns early at line 5375 without calling path_put(&result_path). However, since filename_lookup() returned -ENOENT at line 5372, the result_path was never successfully initialized and doesn't hold a reference. This is correct behavior, but the code flow could be clearer. Consider adding a comment to clarify that result_path is not initialized when filename_lookup returns -ENOENT.

Suggested change
if (error)
if (error)
/* No need to call path_put(&result_path) here:
* result_path was never initialized when filename_lookup()
* returns -ENOENT.
*/

Copilot uses AI. Check for mistakes.
return error;
if (unlikely(type != LAST_NORM)) {
path_put(&parent);
return -EINVAL;
}
/* Duplicate the filename string to avoid dangling pointer */
result_path_last->last = kstrdup((const char *)last.name, GFP_KERNEL);
if (!result_path_last->last) {
path_put(&parent);
return -ENOMEM;
}
result_path_last->path = parent;
return 0;
}

if (error)
return error;

dentry = lookup_one_qstr_excl(&last, parent_path.dentry, 0);
if (!IS_ERR(dentry)) {
result_path->mnt = parent_path.mnt;
result_path->dentry = dentry;
path_get(result_path); // Increment reference count
dput(dentry);
} else {
// If the file does not exist, use the parent directory
*result_path = parent_path;
path_get(result_path);
}

path_put(&parent_path);
result_path_last->last = NULL;
result_path_last->path = result_path;
return 0;
}
#else
int deepin_get_path_for_err_notify(int dfd, struct filename *name,
struct path *result_path)
int deepin_lookup_path_or_parent(int dfd, struct filename *name,
unsigned int flags,
struct deepin_path_last *result_path_last)
{
return -EOPNOTSUPP;
}
Expand Down
Loading
Loading