Hi,
Google has been using an out-of-tree mechanism for error notification in
Ext4 and we decided it is time to push for an upstream solution. This
would surely fit on top of David's notification work.
This patchset is an attempt to restart that discussion. It forward ports
some code from David on top of Linus tree, adds features to
watch_queue and implements ext4 support.
The new notifications are designed after ext4 messages, so it exposes
notifications types to fit that filesystem, but it doesn't change much
to other filesystems, so it should be easily extensible.
I'm aware of the discussion around fsinfo, but I'd like to ask if there
are other missing pieces and what we could do to help that work go
upstream. From a previous mailing list discussion, Linus complained
about lack of users as a main reason for it to not be merged, so hey! :)
In addition, I'd like to ask for feedback on the current implementation,
specifically regarding the passing of extra unformatted information at
the end of the notification and the ext4 support.
The work, as shared on this patchset can be found at:
https://gitlab.collabora.com/krisman/linux.git -b ext4-error-notifications
And there is an example code at:
https://gitlab.collabora.com/krisman/ext4-watcher
I'm Cc'ing Khazhismel Kumykov, from Google, who can provide more
information about their use case, if requested.
David Howells (3):
watch_queue: Make watch_sizeof() check record size
security: Add hooks to rule on setting a watch for superblock
vfs: Add superblock notifications
Gabriel Krisman Bertazi (4):
watch_queue: Support a text field at the end of the notification
vfs: Include origin of the SB error notification
fs: Add more superblock error subtypes
ext4: Implement SB error notification through watch_sb
arch/x86/entry/syscalls/syscall_32.tbl | 1 +
arch/x86/entry/syscalls/syscall_64.tbl | 1 +
fs/Kconfig | 12 ++
fs/ext4/super.c | 32 +++-
fs/super.c | 127 +++++++++++++++
include/linux/fs.h | 207 +++++++++++++++++++++++++
include/linux/lsm_hook_defs.h | 1 +
include/linux/lsm_hooks.h | 4 +
include/linux/security.h | 13 ++
include/linux/syscalls.h | 2 +
include/linux/watch_queue.h | 21 ++-
include/uapi/asm-generic/unistd.h | 4 +-
include/uapi/linux/watch_queue.h | 68 +++++++-
kernel/sys_ni.c | 3 +
kernel/watch_queue.c | 29 +++-
security/security.c | 7 +
16 files changed, 514 insertions(+), 18 deletions(-)
--
2.28.0
From: David Howells <[email protected]>
Make watch_sizeof() give a build error if the size of the struct won't fit
into the size field in the header.
Reported-by: Miklos Szeredi <[email protected]>
Signed-off-by: David Howells <[email protected]>
[Rebase to 5.10]
Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
---
include/linux/watch_queue.h | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/include/linux/watch_queue.h b/include/linux/watch_queue.h
index c994d1b2cdba..f1086d12cd03 100644
--- a/include/linux/watch_queue.h
+++ b/include/linux/watch_queue.h
@@ -120,7 +120,12 @@ static inline void remove_watch_list(struct watch_list *wlist, u64 id)
* watch_sizeof - Calculate the information part of the size of a watch record,
* given the structure size.
*/
-#define watch_sizeof(STRUCT) (sizeof(STRUCT) << WATCH_INFO_LENGTH__SHIFT)
+#define watch_sizeof(STRUCT) \
+ ({ \
+ size_t max = WATCH_INFO_LENGTH >> WATCH_INFO_LENGTH__SHIFT; \
+ BUILD_BUG_ON(sizeof(STRUCT) > max); \
+ sizeof(STRUCT) << WATCH_INFO_LENGTH__SHIFT; \
+ })
#else
static inline int watch_queue_init(struct pipe_inode_info *pipe)
--
2.28.0
From: David Howells <[email protected]>
Add security hooks that will allow an LSM to rule on whether or not a watch
may be set for a supperblock.
Signed-off-by: David Howells <[email protected]>
[Drop mount and key changes. Rebase to mainline]
Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
---
include/linux/lsm_hook_defs.h | 1 +
include/linux/lsm_hooks.h | 4 ++++
include/linux/security.h | 13 +++++++++++++
security/security.c | 7 +++++++
4 files changed, 25 insertions(+)
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 32a940117e7a..8fa8533598bc 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -261,6 +261,7 @@ LSM_HOOK(int, 0, inode_getsecctx, struct inode *inode, void **ctx,
#if defined(CONFIG_SECURITY) && defined(CONFIG_WATCH_QUEUE)
LSM_HOOK(int, 0, post_notification, const struct cred *w_cred,
const struct cred *cred, struct watch_notification *n)
+LSM_HOOK(int, 0, watch_sb, struct super_block *sb)
#endif /* CONFIG_SECURITY && CONFIG_WATCH_QUEUE */
#if defined(CONFIG_SECURITY) && defined(CONFIG_KEY_NOTIFICATIONS)
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 8814e3d5952d..325f50bea1ba 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1475,6 +1475,10 @@
* @w_cred: The credentials of the whoever set the watch.
* @cred: The event-triggerer's credentials
* @n: The notification being posted
+ * @watch_sb:
+ * Check to see if a process is allowed to watch for event notifications
+ * from a superblock.
+ * @sb: The superblock to watch.
*
* @watch_key:
* Check to see if a process is allowed to watch for event notifications
diff --git a/include/linux/security.h b/include/linux/security.h
index bc2725491560..078e11a8872a 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -456,6 +456,11 @@ int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen);
int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen);
int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen);
int security_locked_down(enum lockdown_reason what);
+
+#ifdef CONFIG_WATCH_QUEUE
+int security_watch_sb(struct super_block *sb);
+#endif /* CONFIG_WATCH_QUEUE */
+
#else /* CONFIG_SECURITY */
static inline int call_blocking_lsm_notifier(enum lsm_event event, void *data)
@@ -1304,6 +1309,14 @@ static inline int security_locked_down(enum lockdown_reason what)
{
return 0;
}
+
+#ifdef CONFIG_WATCH_QUEUE
+static inline int security_watch_sb(struct super_block *sb)
+{
+ return 0;
+}
+#endif /* CONFIG_WATCH_QUEUE */
+
#endif /* CONFIG_SECURITY */
#if defined(CONFIG_SECURITY) && defined(CONFIG_WATCH_QUEUE)
diff --git a/security/security.c b/security/security.c
index a28045dc9e7f..a5581aadc644 100644
--- a/security/security.c
+++ b/security/security.c
@@ -2068,12 +2068,19 @@ int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen)
EXPORT_SYMBOL(security_inode_getsecctx);
#ifdef CONFIG_WATCH_QUEUE
+
int security_post_notification(const struct cred *w_cred,
const struct cred *cred,
struct watch_notification *n)
{
return call_int_hook(post_notification, 0, w_cred, cred, n);
}
+
+int security_watch_sb(struct super_block *sb)
+{
+ return call_int_hook(watch_sb, 0, sb);
+}
+
#endif /* CONFIG_WATCH_QUEUE */
#ifdef CONFIG_KEY_NOTIFICATIONS
--
2.28.0
Expose new SB notification subtype for warnings, errors and general
messages. This is modeled after the information exposed by ext4, but
should be the same for other filesystems.
Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
---
include/linux/fs.h | 118 +++++++++++++++++++++++++++++++
include/uapi/linux/watch_queue.h | 33 +++++++++
2 files changed, 151 insertions(+)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index be9f7b480f50..8a11aed95798 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3513,6 +3513,28 @@ static inline void notify_sb(struct super_block *s,
#endif
}
+/**
+ * notify_sb_msg: Post superblock message.
+ * @s: The superblock the notification is about.
+ * @fmt: Formating string for extra information appended to the notification
+ * @args: arguments for extra information string appended to the notification
+ */
+static inline void notify_sb_msg(struct super_block *s, const char *fmt, va_list *args)
+{
+#ifdef CONFIG_SB_NOTIFICATIONS
+ if (unlikely(s->s_watchers)) {
+ struct superblock_msg_notification n = {
+ .s.watch.type = WATCH_TYPE_SB_NOTIFY,
+ .s.watch.subtype = NOTIFY_SUPERBLOCK_MSG,
+ .s.watch.info = watch_sizeof(n),
+ .s.sb_id = s->s_unique_id,
+ };
+
+ post_sb_notification(s, &n.s, fmt, args);
+ }
+#endif
+}
+
/**
* notify_sb_error: Post superblock error notification.
* @s: The superblock the notification is about.
@@ -3546,6 +3568,35 @@ static inline int notify_sb_error(struct super_block *s, const char *function,
return error;
}
+/**
+ * notify_sb_warning: Post superblock warning notification.
+ * @s: The superblock the notification is about.
+ * @function: function name reported as source of the warning.
+ * @line: source code line reported as source of the warning.
+ * @fmt: Formating string for extra information appended to the notification
+ * @args: arguments for extra information string appended to the notification
+ */
+static inline void notify_sb_warning(struct super_block *s, const char *function,
+ int line, const char *fmt, va_list *args)
+{
+#ifdef CONFIG_SB_NOTIFICATIONS
+ if (unlikely(s->s_watchers)) {
+ struct superblock_error_notification n = {
+ .s.watch.type = WATCH_TYPE_SB_NOTIFY,
+ .s.watch.subtype = NOTIFY_SUPERBLOCK_WARNING,
+ .s.watch.info = watch_sizeof(n),
+ .s.sb_id = s->s_unique_id,
+ .line = line,
+ };
+
+ memcpy(&n.function, function, SB_NOTIFICATION_FNAME_LEN);
+ n.function[SB_NOTIFICATION_FNAME_LEN-1] = '\0';
+
+ post_sb_notification(s, &n.s, fmt, args);
+ }
+#endif
+}
+
/**
* notify_sb_EDQUOT: Post superblock quota overrun notification.
* @s: The superblock the notification is about.
@@ -3567,4 +3618,71 @@ static inline int notify_sb_EQDUOT(struct super_block *s)
return -EDQUOT;
}
+/**
+ * notify_sb_inode_error: Post superblock inode error notification.
+ * @s: The superblock the notification is about.
+ * @function: function name reported as source of the warning.
+ * @line: source code line reported as source of the warning.
+ * @error: The error number to be recorded.
+ * @ino: The inode number being accessed.
+ * @block: The block being accessed.
+ * @fmt: Formating string for extra information appended to the notification
+ * @args: arguments for extra information string appended to the notification
+ */
+static inline int notify_sb_inode_error(struct super_block *s, const char *function,
+ int line, int error, u64 ino, u64 block,
+ const char *fmt, va_list *args)
+{
+#ifdef CONFIG_SB_NOTIFICATIONS
+ if (unlikely(s->s_watchers)) {
+ struct superblock_inode_error_notification n = {
+ .s.watch.type = WATCH_TYPE_SB_NOTIFY,
+ .s.watch.subtype = NOTIFY_SUPERBLOCK_INODE_ERROR,
+ .s.watch.info = watch_sizeof(n),
+ .s.sb_id = s->s_unique_id,
+ .error_number = error,
+ .error_cookie = 0,
+ .line = line,
+ .inode = ino,
+ .block = block
+ };
+
+ memcpy(&n.function, function, SB_NOTIFICATION_FNAME_LEN);
+ n.function[SB_NOTIFICATION_FNAME_LEN-1] = '\0';
+
+ post_sb_notification(s, &n.s, fmt, args);
+ }
+#endif
+ return error;
+}
+
+/**
+ * notify_sb_inode_warning: Post superblock inode warning notification.
+ * @s: The superblock the notification is about.
+ * @function: function name reported as source of the warning.
+ * @line: source code line reported as source of the warning.
+ * @ino: The inode number being accessed.
+ * @block: The block being accessed.
+ * @fmt: Formating string for extra information appended to the notification
+ * @args: arguments for extra information string appended to the notification
+ */
+static inline void notify_sb_inode_warning(struct super_block *s, const char *function,
+ int line, u64 ino, u64 block, const char *fmt,
+ va_list *args)
+{
+#ifdef CONFIG_SB_NOTIFICATIONS
+ if (unlikely(s->s_watchers)) {
+ struct superblock_inode_warning_notification n = {
+ .s.watch.type = WATCH_TYPE_SB_NOTIFY,
+ .s.watch.subtype = NOTIFY_SUPERBLOCK_INODE_WARNING,
+ .s.watch.info = watch_sizeof(n),
+ .s.sb_id = s->s_unique_id,
+ .inode = ino,
+ .block = block
+ };
+
+ post_sb_notification(s, &n.s, fmt, args);
+ }
+#endif
+}
#endif /* _LINUX_FS_H */
diff --git a/include/uapi/linux/watch_queue.h b/include/uapi/linux/watch_queue.h
index d0a45a4ded7d..6bfe35dc7b5d 100644
--- a/include/uapi/linux/watch_queue.h
+++ b/include/uapi/linux/watch_queue.h
@@ -110,6 +110,10 @@ enum superblock_notification_type {
NOTIFY_SUPERBLOCK_ERROR = 1, /* Error in filesystem or blockdev */
NOTIFY_SUPERBLOCK_EDQUOT = 2, /* EDQUOT notification */
NOTIFY_SUPERBLOCK_NETWORK = 3, /* Network status change */
+ NOTIFY_SUPERBLOCK_INODE_ERROR = 4, /* Inode Error */
+ NOTIFY_SUPERBLOCK_WARNING = 5, /* Filesystem warning */
+ NOTIFY_SUPERBLOCK_INODE_WARNING = 6, /* Filesystem inode warning */
+ NOTIFY_SUPERBLOCK_MSG = 7, /* Filesystem message */
};
#define NOTIFY_SUPERBLOCK_IS_NOW_RO WATCH_INFO_FLAG_0 /* Superblock changed to R/O */
@@ -134,4 +138,33 @@ struct superblock_error_notification {
char desc[0];
};
+struct superblock_msg_notification {
+ struct superblock_notification s; /* subtype = notify_superblock_msg */
+ char desc[0];
+};
+
+struct superblock_warning_notification {
+ struct superblock_notification s; /* subtype = notify_superblock_warning */
+ char function[SB_NOTIFICATION_FNAME_LEN];
+ __u16 line;
+ char desc[0];
+};
+
+struct superblock_inode_error_notification {
+ struct superblock_notification s; /* subtype = notify_superblock_inode_error */
+ __u32 error_number;
+ __u32 error_cookie;
+ char function[SB_NOTIFICATION_FNAME_LEN];
+ __u16 line;
+ __u64 inode;
+ __u64 block;
+ char desc[0];
+};
+
+struct superblock_inode_warning_notification {
+ struct superblock_notification s; /* subtype = notify_superblock_inode_warning */
+ __u64 inode;
+ __u64 block;
+ char desc[0];
+};
#endif /* _UAPI_LINUX_WATCH_QUEUE_H */
--
2.28.0
This follows the same implementation of ext4 error reporting via dmesg,
but expose that information via the new watch_queue notifications API.
Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
---
fs/ext4/super.c | 32 ++++++++++++++++++++++++--------
1 file changed, 24 insertions(+), 8 deletions(-)
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 8b2736283481..ad96cf4bf6a5 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -542,15 +542,17 @@ void __ext4_error(struct super_block *sb, const char *function,
return;
trace_ext4_error(sb, function, line);
+ va_start(args, fmt);
if (ext4_error_ratelimit(sb)) {
- va_start(args, fmt);
vaf.fmt = fmt;
vaf.va = &args;
printk(KERN_CRIT
"EXT4-fs error (device %s): %s:%d: comm %s: %pV\n",
sb->s_id, function, line, current->comm, &vaf);
- va_end(args);
}
+ notify_sb_error(sb, function, line, error, fmt, &args);
+ va_end(args);
+
save_error_info(sb, error, 0, block, function, line);
ext4_handle_error(sb);
}
@@ -566,8 +568,8 @@ void __ext4_error_inode(struct inode *inode, const char *function,
return;
trace_ext4_error(inode->i_sb, function, line);
+ va_start(args, fmt);
if (ext4_error_ratelimit(inode->i_sb)) {
- va_start(args, fmt);
vaf.fmt = fmt;
vaf.va = &args;
if (block)
@@ -580,8 +582,11 @@ void __ext4_error_inode(struct inode *inode, const char *function,
"inode #%lu: comm %s: %pV\n",
inode->i_sb->s_id, function, line, inode->i_ino,
current->comm, &vaf);
- va_end(args);
}
+ notify_sb_inode_error(inode->i_sb, function, line, error, inode->i_ino, block,
+ fmt, &args);
+ va_end(args);
+
save_error_info(inode->i_sb, error, inode->i_ino, block,
function, line);
ext4_handle_error(inode->i_sb);
@@ -600,11 +605,11 @@ void __ext4_error_file(struct file *file, const char *function,
return;
trace_ext4_error(inode->i_sb, function, line);
+ va_start(args, fmt);
if (ext4_error_ratelimit(inode->i_sb)) {
path = file_path(file, pathname, sizeof(pathname));
if (IS_ERR(path))
path = "(unknown)";
- va_start(args, fmt);
vaf.fmt = fmt;
vaf.va = &args;
if (block)
@@ -619,8 +624,11 @@ void __ext4_error_file(struct file *file, const char *function,
"comm %s: path %s: %pV\n",
inode->i_sb->s_id, function, line, inode->i_ino,
current->comm, path, &vaf);
- va_end(args);
}
+ notify_sb_inode_error(inode->i_sb, function, line, EFSCORRUPTED,
+ inode->i_ino, block, fmt, &args);
+ va_end(args);
+
save_error_info(inode->i_sb, EFSCORRUPTED, inode->i_ino, block,
function, line);
ext4_handle_error(inode->i_sb);
@@ -690,6 +698,8 @@ void __ext4_std_error(struct super_block *sb, const char *function,
sb->s_id, function, line, errstr);
}
+ notify_sb_error(sb, function, line, errno, errstr, NULL);
+
save_error_info(sb, -errno, 0, 0, function, line);
ext4_handle_error(sb);
}
@@ -719,6 +729,7 @@ void __ext4_abort(struct super_block *sb, const char *function,
vaf.va = &args;
printk(KERN_CRIT "EXT4-fs error (device %s): %s:%d: %pV\n",
sb->s_id, function, line, &vaf);
+ notify_sb_error(sb, function, line, error, fmt, &args);
va_end(args);
if (sb_rdonly(sb) == 0) {
@@ -752,6 +763,7 @@ void __ext4_msg(struct super_block *sb,
vaf.fmt = fmt;
vaf.va = &args;
printk("%sEXT4-fs (%s): %pV\n", prefix, sb->s_id, &vaf);
+ notify_sb_msg(sb, fmt, &args);
va_end(args);
}
@@ -776,6 +788,7 @@ void __ext4_warning(struct super_block *sb, const char *function,
vaf.va = &args;
printk(KERN_WARNING "EXT4-fs warning (device %s): %s:%d: %pV\n",
sb->s_id, function, line, &vaf);
+ notify_sb_warning(sb, function, line, fmt, &args);
va_end(args);
}
@@ -794,6 +807,7 @@ void __ext4_warning_inode(const struct inode *inode, const char *function,
printk(KERN_WARNING "EXT4-fs warning (device %s): %s:%d: "
"inode #%lu: comm %s: %pV\n", inode->i_sb->s_id,
function, line, inode->i_ino, current->comm, &vaf);
+ notify_sb_inode_warning(inode->i_sb, function, line, inode->i_ino, 0, fmt, &args);
va_end(args);
}
@@ -813,8 +827,8 @@ __acquires(bitlock)
trace_ext4_error(sb, function, line);
__save_error_info(sb, EFSCORRUPTED, ino, block, function, line);
+ va_start(args, fmt);
if (ext4_error_ratelimit(sb)) {
- va_start(args, fmt);
vaf.fmt = fmt;
vaf.va = &args;
printk(KERN_CRIT "EXT4-fs error (device %s): %s:%d: group %u, ",
@@ -825,8 +839,10 @@ __acquires(bitlock)
printk(KERN_CONT "block %llu:",
(unsigned long long) block);
printk(KERN_CONT "%pV\n", &vaf);
- va_end(args);
}
+ notify_sb_inode_error(sb, function, line, EFSCORRUPTED,
+ ino, block, fmt, &args);
+ va_end(args);
if (test_opt(sb, WARN_ON_ERROR))
WARN_ON_ONCE(1);
--
2.28.0
This allow notifications to send text information to userspace without
having to copy it to a temporary buffer to then copy to the ring. One
use case to pass text information in notifications is for error
reporting, where more debug information might be needed, but we don't
want to explode the number of subtypes of notifications. For instance,
ext4 can have a single inode error notification subtype, and pass more
information on the cause of the error in this field.
Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
---
include/linux/watch_queue.h | 14 ++++++++++++--
kernel/watch_queue.c | 29 ++++++++++++++++++++++++-----
2 files changed, 36 insertions(+), 7 deletions(-)
diff --git a/include/linux/watch_queue.h b/include/linux/watch_queue.h
index f1086d12cd03..2f5a7446bca6 100644
--- a/include/linux/watch_queue.h
+++ b/include/linux/watch_queue.h
@@ -79,7 +79,7 @@ struct watch_list {
extern void __post_watch_notification(struct watch_list *,
struct watch_notification *,
const struct cred *,
- u64);
+ u64, const char*, va_list*);
extern struct watch_queue *get_watch_queue(int);
extern void put_watch_queue(struct watch_queue *);
extern void init_watch(struct watch *, struct watch_queue *);
@@ -105,7 +105,17 @@ static inline void post_watch_notification(struct watch_list *wlist,
u64 id)
{
if (unlikely(wlist))
- __post_watch_notification(wlist, n, cred, id);
+ __post_watch_notification(wlist, n, cred, id, NULL, NULL);
+}
+
+static inline void post_watch_notification_string(struct watch_list *wlist,
+ struct watch_notification *n,
+ const struct cred *cred,
+ u64 id, const char *fmt,
+ va_list *args)
+{
+ if (unlikely(wlist))
+ __post_watch_notification(wlist, n, cred, id, fmt, args);
}
static inline void remove_watch_list(struct watch_list *wlist, u64 id)
diff --git a/kernel/watch_queue.c b/kernel/watch_queue.c
index 0ef8f65bd2d7..81b7970521a9 100644
--- a/kernel/watch_queue.c
+++ b/kernel/watch_queue.c
@@ -70,13 +70,15 @@ static const struct pipe_buf_operations watch_queue_pipe_buf_ops = {
* Post a notification to a watch queue.
*/
static bool post_one_notification(struct watch_queue *wqueue,
- struct watch_notification *n)
+ struct watch_notification *n,
+ const char *fmt, va_list *args)
{
void *p;
struct pipe_inode_info *pipe = wqueue->pipe;
struct pipe_buffer *buf;
struct page *page;
unsigned int head, tail, mask, note, offset, len;
+ int wlen = 0;
bool done = false;
if (!pipe)
@@ -102,6 +104,23 @@ static bool post_one_notification(struct watch_queue *wqueue,
get_page(page);
len = n->info & WATCH_INFO_LENGTH;
p = kmap_atomic(page);
+ /*
+ * Write the tail description before the actual header, because
+ * the string needs to be generated to calculate the final
+ * notification size, that is passed in the header.
+ */
+ if (fmt) {
+ wlen = vscnprintf(p + offset + len, WATCH_INFO_LENGTH - len,
+ fmt, args? *args : NULL);
+ wlen += 1; /* vscnprintf doesn't include '\0' */
+ if (wlen > 0) {
+ n->info = n->info & ~WATCH_INFO_LENGTH;
+ n->info |= (len + wlen) & WATCH_INFO_LENGTH;
+ } else {
+ /* Drop errors when writting the extra string. */
+ wlen = 0;
+ }
+ }
memcpy(p + offset, n, len);
kunmap_atomic(p);
@@ -110,7 +129,7 @@ static bool post_one_notification(struct watch_queue *wqueue,
buf->private = (unsigned long)wqueue;
buf->ops = &watch_queue_pipe_buf_ops;
buf->offset = offset;
- buf->len = len;
+ buf->len = (len + wlen);
buf->flags = PIPE_BUF_FLAG_WHOLE;
pipe->head = head + 1;
@@ -175,7 +194,7 @@ static bool filter_watch_notification(const struct watch_filter *wf,
void __post_watch_notification(struct watch_list *wlist,
struct watch_notification *n,
const struct cred *cred,
- u64 id)
+ u64 id, const char *fmt, va_list *args)
{
const struct watch_filter *wf;
struct watch_queue *wqueue;
@@ -202,7 +221,7 @@ void __post_watch_notification(struct watch_list *wlist,
if (security_post_notification(watch->cred, cred, n) < 0)
continue;
- post_one_notification(wqueue, n);
+ post_one_notification(wqueue, n, fmt, args);
}
rcu_read_unlock();
@@ -522,7 +541,7 @@ int remove_watch_from_object(struct watch_list *wlist, struct watch_queue *wq,
* protecting *wqueue from deallocation.
*/
if (wqueue) {
- post_one_notification(wqueue, &n.watch);
+ post_one_notification(wqueue, &n.watch, NULL, NULL);
spin_lock_bh(&wqueue->lock);
--
2.28.0
When reporting a filesystem error, we really need to know where the
error came from, therefore, include "function:line" information in the
notification sent to userspace. There is no current users of notify_sb
in the kernel, so there are no callers to update.
Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
---
include/linux/fs.h | 10 ++++++++--
include/uapi/linux/watch_queue.h | 3 +++
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index d24905e10623..be9f7b480f50 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3516,12 +3516,14 @@ static inline void notify_sb(struct super_block *s,
/**
* notify_sb_error: Post superblock error notification.
* @s: The superblock the notification is about.
+ * @function: function name reported as source of the warning.
+ * @line: source code line reported as source of the warning.
* @error: The error number to be recorded.
* @fmt: Formating string for extra information appended to the notification
* @args: arguments for extra information string appended to the notification
*/
-static inline int notify_sb_error(struct super_block *s, int error,
- const char *fmt, va_list *args)
+static inline int notify_sb_error(struct super_block *s, const char *function,
+ int line, int error, const char *fmt, va_list *args)
{
#ifdef CONFIG_SB_NOTIFICATIONS
if (unlikely(s->s_watchers)) {
@@ -3532,8 +3534,12 @@ static inline int notify_sb_error(struct super_block *s, int error,
.s.sb_id = s->s_unique_id,
.error_number = error,
.error_cookie = 0,
+ .line = line,
};
+ memcpy(&n.function, function, SB_NOTIFICATION_FNAME_LEN);
+ n.function[SB_NOTIFICATION_FNAME_LEN-1] = '\0';
+
post_sb_notification(s, &n.s, fmt, args);
}
#endif
diff --git a/include/uapi/linux/watch_queue.h b/include/uapi/linux/watch_queue.h
index 5899936534f4..d0a45a4ded7d 100644
--- a/include/uapi/linux/watch_queue.h
+++ b/include/uapi/linux/watch_queue.h
@@ -114,6 +114,7 @@ enum superblock_notification_type {
#define NOTIFY_SUPERBLOCK_IS_NOW_RO WATCH_INFO_FLAG_0 /* Superblock changed to R/O */
+#define SB_NOTIFICATION_FNAME_LEN 30
/*
* Superblock notification record.
* - watch.type = WATCH_TYPE_MOUNT_NOTIFY
@@ -128,6 +129,8 @@ struct superblock_error_notification {
struct superblock_notification s; /* subtype = notify_superblock_error */
__u32 error_number;
__u32 error_cookie;
+ char function[SB_NOTIFICATION_FNAME_LEN];
+ __u16 line;
char desc[0];
};
--
2.28.0
On Tue, Oct 20, 2020 at 03:15:42PM -0400, Gabriel Krisman Bertazi wrote:
> diff --git a/include/uapi/linux/watch_queue.h b/include/uapi/linux/watch_queue.h
> index d0a45a4ded7d..6bfe35dc7b5d 100644
> --- a/include/uapi/linux/watch_queue.h
> +++ b/include/uapi/linux/watch_queue.h
> @@ -110,6 +110,10 @@ enum superblock_notification_type {
> NOTIFY_SUPERBLOCK_ERROR = 1, /* Error in filesystem or blockdev */
> NOTIFY_SUPERBLOCK_EDQUOT = 2, /* EDQUOT notification */
> NOTIFY_SUPERBLOCK_NETWORK = 3, /* Network status change */
> + NOTIFY_SUPERBLOCK_INODE_ERROR = 4, /* Inode Error */
> + NOTIFY_SUPERBLOCK_WARNING = 5, /* Filesystem warning */
> + NOTIFY_SUPERBLOCK_INODE_WARNING = 6, /* Filesystem inode warning */
> + NOTIFY_SUPERBLOCK_MSG = 7, /* Filesystem message */
> };
Hmm, I wonder if this is the right break down. In ext4 we have
ext4_error() and ext4_error_inode(), but that's just a convenience so
that if there is an error number, we can log information relating to
the inode. It's unclear if we need to break apart *_WARNING and
*INODE_WARNING in the notification types. So I'd suggest dropping
*_INODE_ERROR and *_INODE_WARNING and let those get subsumed into
*_ERROR and *_WARNING. We can include the __64 for block and inode
numbers for *_ERROR and _*WARNING, which can be non-zero if they are
available for a particular notification.
I *do* thnk we should separate out file system error and blockdev
warnings, however. So maybe NOTIFY_SUPERBLOCK_ERROR should be
redifined to mean only "file system level error" and we should add a
NOTIFY_SUPERBLOCK_EIO for an I/O errors coming from the block device.
For that notification type, we can add a __u8 or __u32 containing the
BLK_STS_* errors.
I suspect in the future we should also consider a new block device
notification scheme, where we can provide more detailed information
such as SCSI sense codes, etc. But that's a separable feature, I
think.
Cheers,
- Ted