mirror of
https://github.com/torvalds/linux.git
synced 2026-11-07 06:17:02 +01:00
seq_buf: Clear what a writer did not claim when a seq_buf overflows
Using seq_buf_set_overflow() would leave the bytes between "len" and "size" untouched, so if seq_buf_str() is used on an overflowed seq_buf, those bytes may be exposed. For any paths that don't claim partially written bytes, by setting "len = size" before calling seq_buf_set_overflow(), wipe the unclaimed bytes. The seq_buf_puts() and related APIs already claim those bytes now, so only the unclaimed cases remain. A specific example of this was seq_buf_path() which uses d_path() and would write to the tail before discovering it was out of space, and would correctly mark a seq_buf as overflowed, but the path fragment would be left over. Clear from len to the end of the buffer in seq_buf_set_overflow(), which every overflow goes through, including seq_buf_commit() with a negative count. Add a test that fills a seq_buf, leaves it too little room for a path, and checks that nothing of the path is left in the buffer. The tests run before anything writable is mounted, so it takes its file from shmem. seq_buf_path() was not exported, so the test failed to link as a module. Export it. Tests passed under qemu on ARCH=x86_64 with GCC 16.2.0 and CONFIG_KASAN=y, and on big-endian ARCH=s390 with GCC s390x-linux-gnu 16.2.0, and the test builds as a module (CONFIG_SEQ_BUF_KUNIT_TEST=m). Assisted-by: LLM Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Link: https://patch.msgid.link/20261005155708.1471260-5-kees@kernel.org Signed-off-by: Kees Cook <kees@kernel.org>
This commit is contained in:
parent
505a4aeea2
commit
ea5666e567
|
|
@ -5,6 +5,7 @@
|
|||
#include <linux/bug.h>
|
||||
#include <linux/minmax.h>
|
||||
#include <linux/seq_file.h>
|
||||
#include <linux/string.h>
|
||||
#include <linux/types.h>
|
||||
|
||||
/*
|
||||
|
|
@ -58,12 +59,15 @@ seq_buf_has_overflowed(struct seq_buf *s)
|
|||
/*
|
||||
* Mark @s as overflowed, which discards the length of what it holds. The
|
||||
* bytes up to its last one are the string from then on, as that is where
|
||||
* seq_buf_str() terminates it, so a caller that could not fill the buffer
|
||||
* has to NUL them itself before calling this.
|
||||
* seq_buf_str() terminates it, so clear whatever was not written: a writer
|
||||
* sets len to how much it filled, and anything past that was never adopted.
|
||||
*/
|
||||
static inline void
|
||||
seq_buf_set_overflow(struct seq_buf *s)
|
||||
{
|
||||
if (s->len < s->size)
|
||||
memset(s->buffer + s->len, 0, s->size - s->len);
|
||||
|
||||
s->len = s->size + 1;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -76,6 +76,8 @@ int seq_buf_vprintf(struct seq_buf *s, const char *fmt, va_list args)
|
|||
s->len += len;
|
||||
return 0;
|
||||
}
|
||||
/* vsnprintf() wrote as much as fits, so none of it is stale */
|
||||
s->len = s->size;
|
||||
}
|
||||
seq_buf_set_overflow(s);
|
||||
return -1;
|
||||
|
|
@ -164,6 +166,8 @@ int seq_buf_bprintf(struct seq_buf *s, const char *fmt, const u32 *binary)
|
|||
s->len += ret;
|
||||
return 0;
|
||||
}
|
||||
/* bstr_printf() wrote as much as fits, so none of it is stale */
|
||||
s->len = s->size;
|
||||
}
|
||||
seq_buf_set_overflow(s);
|
||||
return -1;
|
||||
|
|
@ -343,6 +347,7 @@ int seq_buf_path(struct seq_buf *s, const struct path *path, const char *esc)
|
|||
|
||||
return res;
|
||||
}
|
||||
EXPORT_SYMBOL_GPL(seq_buf_path);
|
||||
|
||||
/**
|
||||
* seq_buf_to_user - copy the sequence buffer to user space
|
||||
|
|
|
|||
|
|
@ -7,7 +7,9 @@
|
|||
|
||||
#include <kunit/test.h>
|
||||
#include <linux/console.h>
|
||||
#include <linux/fs.h>
|
||||
#include <linux/seq_buf.h>
|
||||
#include <linux/shmem_fs.h>
|
||||
#include <linux/string.h>
|
||||
|
||||
static void seq_buf_init_test(struct kunit *test)
|
||||
|
|
@ -466,6 +468,47 @@ static void seq_buf_do_printk_test(struct kunit *test)
|
|||
KUNIT_EXPECT_EQ(test, seq_buf_printk_empty, 0);
|
||||
}
|
||||
|
||||
/* Long enough that it cannot fit in the room the test leaves for it. */
|
||||
#define SEQ_BUF_TEST_PATH "/seq_buf_kunit_path_name"
|
||||
|
||||
static void seq_buf_path_overflow_test(struct kunit *test)
|
||||
{
|
||||
DECLARE_SEQ_BUF(s, 32);
|
||||
const char *expected = "keep:xxxxxxxxxxxxxxxxxxxxxxx";
|
||||
struct file *file;
|
||||
size_t len;
|
||||
int i;
|
||||
|
||||
/*
|
||||
* The tests run before anything writable is mounted, so take the file
|
||||
* whose path gets printed from shmem, which needs no mount of its own.
|
||||
*/
|
||||
file = shmem_file_setup(SEQ_BUF_TEST_PATH, 0, EMPTY_VMA_FLAGS);
|
||||
if (IS_ERR(file))
|
||||
kunit_skip(test, "cannot create a file to print the path of");
|
||||
|
||||
/* Leave less room than the path needs, so d_path() cannot fit it. */
|
||||
seq_buf_puts(&s, "keep:");
|
||||
len = seq_buf_used(&s);
|
||||
for (i = len; i < 28; i++)
|
||||
seq_buf_putc(&s, 'x');
|
||||
|
||||
KUNIT_EXPECT_EQ(test, seq_buf_path(&s, &file->f_path, "\n"), -1);
|
||||
fput(file);
|
||||
|
||||
KUNIT_EXPECT_TRUE(test, seq_buf_has_overflowed(&s));
|
||||
|
||||
/*
|
||||
* d_path() keeps as much of the path as fits when it does not fit
|
||||
* whole, and seq_buf_str() would hand out that fragment, as it ends
|
||||
* the string at the last byte of an overflowed buffer.
|
||||
*/
|
||||
for (i = 28; i < 32; i++)
|
||||
KUNIT_EXPECT_EQ_MSG(test, s.buffer[i], '\0',
|
||||
"byte %d past the data is not cleared", i);
|
||||
KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), expected);
|
||||
}
|
||||
|
||||
static struct kunit_case seq_buf_test_cases[] = {
|
||||
KUNIT_CASE(seq_buf_init_test),
|
||||
KUNIT_CASE(seq_buf_declare_test),
|
||||
|
|
@ -482,6 +525,7 @@ static struct kunit_case seq_buf_test_cases[] = {
|
|||
KUNIT_CASE(seq_buf_puts_partial_overflow_test),
|
||||
KUNIT_CASE(seq_buf_putmem_partial_overflow_test),
|
||||
KUNIT_CASE(seq_buf_putmem_hex_partial_overflow_test),
|
||||
KUNIT_CASE(seq_buf_path_overflow_test),
|
||||
KUNIT_CASE(seq_buf_do_printk_test),
|
||||
{}
|
||||
};
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user