| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <e4187844aed14d4897f0071dcfce7455@huawei.com>
Date: Tue, 3 Dec 2024 15:21:44 +0000
From: Shiju Jose <shiju.jose@...wei.com>
To: Steven Rostedt <rostedt@...dmis.org>
CC: "dave.jiang@...el.com" <dave.jiang@...el.com>, "dan.j.williams@...el.com"
<dan.j.williams@...el.com>, Jonathan Cameron <jonathan.cameron@...wei.com>,
"alison.schofield@...el.com" <alison.schofield@...el.com>,
"nifan.cxl@...il.com" <nifan.cxl@...il.com>, "vishal.l.verma@...el.com"
<vishal.l.verma@...el.com>, "ira.weiny@...el.com" <ira.weiny@...el.com>,
"dave@...olabs.net" <dave@...olabs.net>, "linux-cxl@...r.kernel.org"
<linux-cxl@...r.kernel.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, Linuxarm <linuxarm@...wei.com>, tanxiaofei
<tanxiaofei@...wei.com>, "Zengtao (B)" <prime.zeng@...ilicon.com>
Subject: RE: [PATCH v4 3/6] cxl/events: Update General Media Event Record to
CXL spec rev 3.1
>-----Original Message-----
>From: Shiju Jose <shiju.jose@...wei.com>
>Sent: 29 November 2024 13:23
>To: Shiju Jose <shiju.jose@...wei.com>; Steven Rostedt <rostedt@...dmis.org>
>Cc: dave.jiang@...el.com; dan.j.williams@...el.com; Jonathan Cameron
><jonathan.cameron@...wei.com>; alison.schofield@...el.com;
>nifan.cxl@...il.com; vishal.l.verma@...el.com; ira.weiny@...el.com;
>dave@...olabs.net; linux-cxl@...r.kernel.org; linux-kernel@...r.kernel.org;
>Linuxarm <linuxarm@...wei.com>; tanxiaofei <tanxiaofei@...wei.com>;
>Zengtao (B) <prime.zeng@...ilicon.com>
>Subject: RE: [PATCH v4 3/6] cxl/events: Update General Media Event Record to
>CXL spec rev 3.1
>
>>-----Original Message-----
>>From: Shiju Jose <shiju.jose@...wei.com>
>>Sent: 28 November 2024 10:02
>>To: Steven Rostedt <rostedt@...dmis.org>
>>Cc: dave.jiang@...el.com; dan.j.williams@...el.com; Jonathan Cameron
>><jonathan.cameron@...wei.com>; alison.schofield@...el.com;
>>nifan.cxl@...il.com; vishal.l.verma@...el.com; ira.weiny@...el.com;
>>dave@...olabs.net; linux-cxl@...r.kernel.org;
>>linux-kernel@...r.kernel.org; Linuxarm <linuxarm@...wei.com>;
>>tanxiaofei <tanxiaofei@...wei.com>; Zengtao (B)
>><prime.zeng@...ilicon.com>
>>Subject: RE: [PATCH v4 3/6] cxl/events: Update General Media Event
>>Record to CXL spec rev 3.1
>>
[...]
>
>Hi Steve,
>
>I debug this case and please find the info, 1. rasdaemon : read() from format
>file return around 1/3rd of the
> actual data in the file only when the total size of the format's file
> is above 4K bytes (page size), For example, in this case, the total size was 4512
>bytes,
> but read only 1674 bytes.
> This partial data resulted tep_parse_event() in libtraceevent failed internally
>in the parse_format()
> and since __parse_event() does not return error when parse_format() fail,
> thus initialization of the event does not fail.
>
> The following solution in rasdaemon solved the issue,
> (provided if no other fix for the above issue with read()),
> 1. read() and accumulate content of format file until EOF reached.
> 2. Increased the buffer size from 4K bytes to 8K bytes.
> 3. May be __parse_event() in libtraceevent and thus tep_parse_event()
>return error
> when parse_format() fail so that the initialization would fail if the input
>data is
> corrupted or partial?
Hi Steve,
Identified the root cause of this issue in the kernel:
The read() function for the event's format file calls seq_read() and seq_read_iter() in the kernel,
which allocates a buffer of PAGE_SIZE (4 KB) for sequential reads. However, it
detects an overflow in the following marked call (seq_has_overflowed()) and
returns with partial data read.
=====================================
File : https://elixir.bootlin.com/linux/v6.13-rc1/source/fs/seq_file.c
ssize_t seq_read_iter(struct kiocb *iocb, struct iov_iter *iter)
{
struct seq_file *m = iocb->ki_filp->private_data;
[...]
/* grab buffer if we didn't have one */
if (!m->buf) {
---------> m->buf = seq_buf_alloc(m->size = PAGE_SIZE);
if (!m->buf)
goto Enomem;
}
// something left in the buffer - copy it out first
[...]
// get a non-empty record in the buffer
m->from = 0;
p = m->op->start(m, &m->index);
while (1) {
err = PTR_ERR(p);
if (!p || IS_ERR(p)) // EOF or an error
break;
[...]
}
// EOF or an error
m->op->stop(m, p);
m->count = 0;
goto Done;
Fill:
// one non-empty record is in the buffer; if they want more,
// try to fit more in, but in any case we need to advance
// the iterator once for every record shown.
while (1) {
size_t offs = m->count;
loff_t pos = m->index;
[...]
err = m->op->show(m, p);
if (err > 0) { // ->show() says "skip it"
m->count = offs;
----------> } else if (err || seq_has_overflowed(m)) {
m->count = offs;
break;
}
}
m->op->stop(m, p);
n = copy_to_iter(m->buf, m->count, iter);
copied += n;
m->count -= n;
m->from = n;
Done:
[...]
goto Done;
}
EXPORT_SYMBOL(seq_read_iter);
====================================
The following fix in the kernel's seq_read_iter() allows userspace to read the
content of the format file etc in one go if the requested size exceeds PAGE_SIZE,
and resolves the parsing error caused by the event's format file exceeding PAGE_SIZE.
====================================
diff --git a/fs/seq_file.c b/fs/seq_file.c index e676c8b0cf5d..f1f1af180562 100644
--- a/fs/seq_file.c
+++ b/fs/seq_file.c
@@ -207,7 +207,11 @@ ssize_t seq_read_iter(struct kiocb *iocb, struct iov_iter *iter)
/* grab buffer if we didn't have one */
if (!m->buf) {
- m->buf = seq_buf_alloc(m->size = PAGE_SIZE);
+ if (iter->count % PAGE_SIZE)
+ m->size = ((iter->count / PAGE_SIZE) + 1) * PAGE_SIZE;
+ else
+ m->size = (iter->count / PAGE_SIZE) * PAGE_SIZE;
+ m->buf = seq_buf_alloc(m->size);
if (!m->buf)
goto Enomem;
}
====================================
Can you suggest which fix is the right one, kernel based fix or rasdaemon based fix (which
mentioned in the previous email)?
Thanks,
Shiju
>>
>>>
>>>>
>>>> I've also attached another format file,
>>>> "format_cxl_general_media_v3.1_full",
>>>> which contains the complete TP_printk() intended.
>>>
[...]
>>>-- Steve
>>
>
Powered by blists - more mailing lists