<div dir="ltr"><div dir="ltr"><div class="gmail_default" style="font-size:small"><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Nov 3, 2020 at 9:10 PM Jan Stancek <<a href="mailto:jstancek@redhat.com">jstancek@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">tst_cgroup_cpuset_read_files() is wrongly using sizeof on pointer<br>
type to figure out length of buffer. On some systems there's<br>
more content in cgroup mems/cpus, which leads to partial read<br>
and subsequent EINVAL on write.<br>
<br>
Also the retval buffer should be zero terminated, since buffer<br>
can be passed uninitialized, subsequently leading to writing garbage<br>
to cgroup and again hitting EINVAL.<br>
<br>
Before:<br>
tst_test.c:1250: TINFO: Timeout per run is 0h 05m 00s<br>
tst_device.c:423: TINFO: No device is mounted at cgroup2<br>
tst_device.c:423: TINFO: No device is mounted at /tmp/cgroup_cst<br>
tst_cgroup.c:100: TINFO: Cgroup(cpuset) v1 mount at /tmp/cgroup_cst success<br>
safe_macros.c:458: TBROK: tst_cgroup.c:449: write(6,0x7fffc0425410,18446744073709551615) failed: EINVAL (22)<br>
tst_cgroup.c:173: TWARN: umount /tmp/cgroup_cst: EBUSY (16)<br>
tst_cgroup.c:175: TWARN: rmdir /tmp/cgroup_cst: EBUSY (16)<br>
tst_cgroup.c:178: TINFO: Cgroup v1 unmount success<br>
<br>
After:<br>
tst_test.c:1250: TINFO: Timeout per run is 0h 05m 00s<br>
tst_device.c:423: TINFO: No device is mounted at cgroup2<br>
tst_device.c:423: TINFO: No device is mounted at /tmp/cgroup_cst<br>
tst_cgroup.c:100: TINFO: Cgroup(cpuset) v1 mount at /tmp/cgroup_cst success<br>
cpuset01.c:83: TPASS: cpuset test pass<br>
tst_cgroup.c:178: TINFO: Cgroup v1 unmount success<br>
<br>
Signed-off-by: Jan Stancek <<a href="mailto:jstancek@redhat.com" target="_blank">jstancek@redhat.com</a>><br>
---<br>
include/tst_cgroup.h | 6 ++++--<br>
lib/tst_cgroup.c | 6 ++++--<br>
testcases/kernel/mem/cpuset/cpuset01.c | 4 ++--<br>
3 files changed, 10 insertions(+), 6 deletions(-)<br>
<br>
diff --git a/include/tst_cgroup.h b/include/tst_cgroup.h<br>
index 77780e0d64c9..bfd8482608c4 100644<br>
--- a/include/tst_cgroup.h<br>
+++ b/include/tst_cgroup.h<br>
@@ -39,7 +39,9 @@ int tst_cgroup_mem_swapacct_enabled(const char *cgroup_dir);<br>
void tst_cgroup_mem_set_maxswap(const char *cgroup_dir, long memsz);<br>
<br>
/* Set of functions to read/write cpuset controller files content */<br>
-void tst_cgroup_cpuset_read_files(const char *cgroup_dir, const char *filename, char *retbuf);<br>
-void tst_cgroup_cpuset_write_files(const char *cgroup_dir, const char *filename, const char *buf);<br>
+void tst_cgroup_cpuset_read_files(const char *cgroup_dir, const char *filename,<br>
+ char *retbuf, size_t retbuf_sz);<br>
+void tst_cgroup_cpuset_write_files(const char *cgroup_dir, const char *filename,<br>
+ const char *buf);<br>
<br>
#endif /* TST_CGROUP_H */<br>
diff --git a/lib/tst_cgroup.c b/lib/tst_cgroup.c<br>
index ba413d874f69..96c9524d2b3a 100644<br>
--- a/lib/tst_cgroup.c<br>
+++ b/lib/tst_cgroup.c<br>
@@ -393,7 +393,8 @@ void tst_cgroup_mem_set_maxswap(const char *cgroup_dir, long memsz)<br>
tst_cgroup_set_knob(cgroup_dir, "memory.swap.max", memsz);<br>
}<br>
<br>
-void tst_cgroup_cpuset_read_files(const char *cgroup_dir, const char *filename, char *retbuf)<br>
+void tst_cgroup_cpuset_read_files(const char *cgroup_dir, const char *filename,<br>
+ char *retbuf, size_t retbuf_sz)<br>
{<br>
int fd;<br>
char *cgroup_new_dir;<br>
@@ -417,7 +418,8 @@ void tst_cgroup_cpuset_read_files(const char *cgroup_dir, const char *filename,<br>
tst_brk(TBROK | TERRNO, "open %s", knob_path);<br>
}<br>
<br>
- if (read(fd, retbuf, sizeof(retbuf)) < 0)<br>
+ memset(retbuf, 0, retbuf_sz);<br>
+ if (read(fd, retbuf, retbuf_sz) < 0)<br>
tst_brk(TBROK | TERRNO, "read %s", knob_path);<br>
<br>
close(fd);<br>
diff --git a/testcases/kernel/mem/cpuset/cpuset01.c b/testcases/kernel/mem/cpuset/cpuset01.c<br>
index f70ae0e486c2..fcd22040f7dd 100644<br>
--- a/testcases/kernel/mem/cpuset/cpuset01.c<br>
+++ b/testcases/kernel/mem/cpuset/cpuset01.c<br>
@@ -51,9 +51,9 @@ static void test_cpuset(void)<br>
unsigned long nmask[MAXNODES / BITS_PER_LONG] = { 0 };<br>
char mems[BUFSIZ], buf[BUFSIZ];<br>
<br>
- tst_cgroup_cpuset_read_files(PATH_TMP_CG_CST, "cpus", buf);<br>
+ tst_cgroup_cpuset_read_files(PATH_TMP_CG_CST, "cpus", buf, sizeof(buf));<br>
tst_cgroup_cpuset_write_files(PATH_TMP_CG_CST, "cpus", buf);<br>
- tst_cgroup_cpuset_read_files(PATH_TMP_CG_CST, "mems", mems);<br>
+ tst_cgroup_cpuset_read_files(PATH_TMP_CG_CST, "mems", mems, sizeof(buf));<br></blockquote><div><br></div>sizeof() is <span class="gmail_default" style="font-size:small">generally </span>used to calculate the size (in bytes) of a data type<span class="gmail_default" style="font-size:small">, e.g sizeof(char *).</span></div><div class="gmail_quote"><span class="gmail_default" style="font-size:small">I think here to pass 'BUFSIZ' directly is better than sizeof(buf).<br></span></div><div class="gmail_quote"><span class="gmail_default" style="font-size:small"><br></span></div><div class="gmail_quote"><span class="gmail_default" style="font-size:small">Otherwise, looks good to me.</span></div><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
tst_cgroup_cpuset_write_files(PATH_TMP_CG_CST, "mems", mems);<br>
tst_cgroup_move_current(PATH_TMP_CG_CST);<br>
<br>
-- <br>
2.18.1<br>
<br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div>Regards,<br></div><div>Li Wang<br></div></div></div></div>