* [BUG] lseek on /proc/meminfo is broken in 4.19.59
@ 2019-08-01 6:16 Sergei Turchanov
2019-08-01 7:11 ` Gao Xiang
0 siblings, 1 reply; 11+ messages in thread
From: Sergei Turchanov @ 2019-08-01 6:16 UTC (permalink / raw)
To: Alexander Viro, linux-fsdevel; +Cc: linux-kernel
Hello!
(I sent this e-mail two weeks ago with no feedback. Does anyone care?
Wrong mailing list? Anything....?)
Seeking (to an offset within file size) in /proc/meminfo is broken in
4.19.59. It does seek to a desired position, but reading from that
position returns the remainder of file and then a whole copy of file.
This doesn't happen with /proc/vmstat or /proc/self/maps for example.
Seeking did work correctly in kernel 4.14.47. So it seems something
broke in the way.
Background: this kind of access pattern (seeking to /proc/meminfo) is
used by libvirt-lxc fuse driver for virtualized view of /proc/meminfo.
So that /proc/meminfo is broken in guests when running kernel 4.19.x.
$ ./test /proc/meminfo 0 # Works as expected
MemTotal: 394907728 kB
MemFree: 173738328 kB
...
DirectMap2M: 13062144 kB
DirectMap1G: 390070272 kB
-----------------------------------------------------------------------
$ ./test 1024 # returns a copy of file after the remainder
Will seek to 1024
Data read at offset 1024
gePages: 0 kB
ShmemHugePages: 0 kB
ShmemPmdMapped: 0 kB
HugePages_Total: 0
HugePages_Free: 0
HugePages_Rsvd: 0
HugePages_Surp: 0
Hugepagesize: 2048 kB
Hugetlb: 0 kB
DirectMap4k: 245204 kB
DirectMap2M: 13062144 kB
DirectMap1G: 390070272 kB
MemTotal: 394907728 kB
MemFree: 173738328 kB
MemAvailable: 379989680 kB
Buffers: 355812 kB
Cached: 207216224 kB
...
DirectMap2M: 13062144 kB
DirectMap1G: 390070272 kB
As you see, after "DirectMap1G:" line, a whole copy of /proc/meminfo
returned by "read".
Test program:
#include <sys/types.h>
#include <sys/stat.h>
#include <unistd.h>
#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>
#define SIZE 1024
char buf[SIZE + 1];
int main(int argc, char *argv[]) {
int fd;
ssize_t rd;
off_t ofs = 0;
if (argc < 2) {
printf("Usage: test <file> [<offset>]\n");
exit(1);
}
if (-1 == (fd = open(argv[1], O_RDONLY))) {
perror("open failed");
exit(1);
}
if (argc > 2) {
ofs = atol(argv[2]);
}
printf("Will seek to %ld\n", ofs);
if (-1 == (lseek(fd, ofs, SEEK_SET))) {
perror("lseek failed");
exit(1);
}
for (;; ofs += rd) {
printf("\n\nData read at offset %ld\n", ofs);
if (-1 == (rd = read(fd, buf, SIZE))) {
perror("read failed");
exit(1);
}
buf[rd] = '\0';
printf(buf);
if (rd < SIZE) {
break;
}
}
return 0;
}
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [BUG] lseek on /proc/meminfo is broken in 4.19.59
2019-08-01 6:16 [BUG] lseek on /proc/meminfo is broken in 4.19.59 Sergei Turchanov
@ 2019-08-01 7:11 ` Gao Xiang
2019-08-01 7:52 ` Sergei Turchanov
2019-08-01 8:09 ` [BUG] lseek on /proc/meminfo is broken in 4.19.59 maybe due to commit 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and interface") Sergei Turchanov
0 siblings, 2 replies; 11+ messages in thread
From: Gao Xiang @ 2019-08-01 7:11 UTC (permalink / raw)
To: Sergei Turchanov; +Cc: Alexander Viro, linux-fsdevel, linux-kernel
Hi,
I just took a glance, maybe due to
commit 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and interface")
I simply reverted it just now and it seems fine... but I haven't digged into this commit.
Maybe you could Cc NeilBrown <neilb@suse.com> for some more advice and
I have no idea whether it's an expected behavior or not...
Thanks,
Gao Xiang
On 2019/8/1 14:16, Sergei Turchanov wrote:
> Hello!
>
> (I sent this e-mail two weeks ago with no feedback. Does anyone care? Wrong mailing list? Anything....?)
>
> Seeking (to an offset within file size) in /proc/meminfo is broken in 4.19.59. It does seek to a desired position, but reading from that position returns the remainder of file and then a whole copy of file. This doesn't happen with /proc/vmstat or /proc/self/maps for example.
>
> Seeking did work correctly in kernel 4.14.47. So it seems something broke in the way.
>
> Background: this kind of access pattern (seeking to /proc/meminfo) is used by libvirt-lxc fuse driver for virtualized view of /proc/meminfo. So that /proc/meminfo is broken in guests when running kernel 4.19.x.
>
> $ ./test /proc/meminfo 0 # Works as expected
>
> MemTotal: 394907728 kB
> MemFree: 173738328 kB
> ...
> DirectMap2M: 13062144 kB
> DirectMap1G: 390070272 kB
>
> -----------------------------------------------------------------------
>
> $ ./test 1024 # returns a copy of file after the remainder
>
> Will seek to 1024
>
>
> Data read at offset 1024
> gePages: 0 kB
> ShmemHugePages: 0 kB
> ShmemPmdMapped: 0 kB
> HugePages_Total: 0
> HugePages_Free: 0
> HugePages_Rsvd: 0
> HugePages_Surp: 0
> Hugepagesize: 2048 kB
> Hugetlb: 0 kB
> DirectMap4k: 245204 kB
> DirectMap2M: 13062144 kB
> DirectMap1G: 390070272 kB
> MemTotal: 394907728 kB
> MemFree: 173738328 kB
> MemAvailable: 379989680 kB
> Buffers: 355812 kB
> Cached: 207216224 kB
> ...
> DirectMap2M: 13062144 kB
> DirectMap1G: 390070272 kB
>
> As you see, after "DirectMap1G:" line, a whole copy of /proc/meminfo returned by "read".
>
> Test program:
>
> #include <sys/types.h>
> #include <sys/stat.h>
> #include <unistd.h>
> #include <fcntl.h>
> #include <stdio.h>
> #include <stdlib.h>
>
> #define SIZE 1024
> char buf[SIZE + 1];
>
> int main(int argc, char *argv[]) {
> int fd;
> ssize_t rd;
> off_t ofs = 0;
>
> if (argc < 2) {
> printf("Usage: test <file> [<offset>]\n");
> exit(1);
> }
>
> if (-1 == (fd = open(argv[1], O_RDONLY))) {
> perror("open failed");
> exit(1);
> }
>
> if (argc > 2) {
> ofs = atol(argv[2]);
> }
> printf("Will seek to %ld\n", ofs);
>
> if (-1 == (lseek(fd, ofs, SEEK_SET))) {
> perror("lseek failed");
> exit(1);
> }
>
> for (;; ofs += rd) {
> printf("\n\nData read at offset %ld\n", ofs);
> if (-1 == (rd = read(fd, buf, SIZE))) {
> perror("read failed");
> exit(1);
> }
> buf[rd] = '\0';
> printf(buf);
> if (rd < SIZE) {
> break;
> }
> }
>
> return 0;
> }
>
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [BUG] lseek on /proc/meminfo is broken in 4.19.59
2019-08-01 7:11 ` Gao Xiang
@ 2019-08-01 7:52 ` Sergei Turchanov
2019-08-01 8:09 ` [BUG] lseek on /proc/meminfo is broken in 4.19.59 maybe due to commit 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and interface") Sergei Turchanov
1 sibling, 0 replies; 11+ messages in thread
From: Sergei Turchanov @ 2019-08-01 7:52 UTC (permalink / raw)
To: Gao Xiang; +Cc: Alexander Viro, linux-fsdevel, linux-kernel
Hi,
Thank you very much for your suggestion. Will certainly do that.
With best regards,
Sergei.
On 01.08.2019 17:11, Gao Xiang wrote:
> Hi,
>
> I just took a glance, maybe due to
> commit 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and interface")
>
> I simply reverted it just now and it seems fine... but I haven't digged into this commit.
>
> Maybe you could Cc NeilBrown <neilb@suse.com> for some more advice and
> I have no idea whether it's an expected behavior or not...
>
> Thanks,
> Gao Xiang
>
> On 2019/8/1 14:16, Sergei Turchanov wrote:
>> Hello!
>>
>> (I sent this e-mail two weeks ago with no feedback. Does anyone care? Wrong mailing list? Anything....?)
>>
>> Seeking (to an offset within file size) in /proc/meminfo is broken in 4.19.59. It does seek to a desired position, but reading from that position returns the remainder of file and then a whole copy of file. This doesn't happen with /proc/vmstat or /proc/self/maps for example.
>>
>> Seeking did work correctly in kernel 4.14.47. So it seems something broke in the way.
>>
>> Background: this kind of access pattern (seeking to /proc/meminfo) is used by libvirt-lxc fuse driver for virtualized view of /proc/meminfo. So that /proc/meminfo is broken in guests when running kernel 4.19.x.
>>
>> $ ./test /proc/meminfo 0 # Works as expected
>>
>> MemTotal: 394907728 kB
>> MemFree: 173738328 kB
>> ...
>> DirectMap2M: 13062144 kB
>> DirectMap1G: 390070272 kB
>>
>> -----------------------------------------------------------------------
>>
>> $ ./test 1024 # returns a copy of file after the remainder
>>
>> Will seek to 1024
>>
>>
>> Data read at offset 1024
>> gePages: 0 kB
>> ShmemHugePages: 0 kB
>> ShmemPmdMapped: 0 kB
>> HugePages_Total: 0
>> HugePages_Free: 0
>> HugePages_Rsvd: 0
>> HugePages_Surp: 0
>> Hugepagesize: 2048 kB
>> Hugetlb: 0 kB
>> DirectMap4k: 245204 kB
>> DirectMap2M: 13062144 kB
>> DirectMap1G: 390070272 kB
>> MemTotal: 394907728 kB
>> MemFree: 173738328 kB
>> MemAvailable: 379989680 kB
>> Buffers: 355812 kB
>> Cached: 207216224 kB
>> ...
>> DirectMap2M: 13062144 kB
>> DirectMap1G: 390070272 kB
>>
>> As you see, after "DirectMap1G:" line, a whole copy of /proc/meminfo returned by "read".
>>
>> Test program:
>>
>> #include <sys/types.h>
>> #include <sys/stat.h>
>> #include <unistd.h>
>> #include <fcntl.h>
>> #include <stdio.h>
>> #include <stdlib.h>
>>
>> #define SIZE 1024
>> char buf[SIZE + 1];
>>
>> int main(int argc, char *argv[]) {
>> int fd;
>> ssize_t rd;
>> off_t ofs = 0;
>>
>> if (argc < 2) {
>> printf("Usage: test <file> [<offset>]\n");
>> exit(1);
>> }
>>
>> if (-1 == (fd = open(argv[1], O_RDONLY))) {
>> perror("open failed");
>> exit(1);
>> }
>>
>> if (argc > 2) {
>> ofs = atol(argv[2]);
>> }
>> printf("Will seek to %ld\n", ofs);
>>
>> if (-1 == (lseek(fd, ofs, SEEK_SET))) {
>> perror("lseek failed");
>> exit(1);
>> }
>>
>> for (;; ofs += rd) {
>> printf("\n\nData read at offset %ld\n", ofs);
>> if (-1 == (rd = read(fd, buf, SIZE))) {
>> perror("read failed");
>> exit(1);
>> }
>> buf[rd] = '\0';
>> printf(buf);
>> if (rd < SIZE) {
>> break;
>> }
>> }
>>
>> return 0;
>> }
>>
>>
>>
^ permalink raw reply [flat|nested] 11+ messages in thread* [BUG] lseek on /proc/meminfo is broken in 4.19.59 maybe due to commit 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and interface")
2019-08-01 7:11 ` Gao Xiang
2019-08-01 7:52 ` Sergei Turchanov
@ 2019-08-01 8:09 ` Sergei Turchanov
2019-08-01 9:14 ` NeilBrown
1 sibling, 1 reply; 11+ messages in thread
From: Sergei Turchanov @ 2019-08-01 8:09 UTC (permalink / raw)
To: NeilBrown; +Cc: Alexander Viro, linux-fsdevel, linux-kernel
Hello!
[
As suggested in previous discussion this behavior may be caused by your
commit 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and interface")
]
Original bug report:
Seeking (to an offset within file size) in /proc/meminfo is broken in 4.19.59. It does seek to a desired position, but reading from that position returns the remainder of file and then a whole copy of file. This doesn't happen with /proc/vmstat or /proc/self/maps for example.
Seeking did work correctly in kernel 4.14.47. So it seems something broke in the way.
Background: this kind of access pattern (seeking to /proc/meminfo) is used by libvirt-lxc fuse driver for virtualized view of /proc/meminfo. So that /proc/meminfo is broken in guests when running kernel 4.19.x.
> On 01.08.2019 17:11, Gao Xiang wrote:
> Hi,
>
> I just took a glance, maybe due to
> commit 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and interface")
>
> I simply reverted it just now and it seems fine... but I haven't digged into this commit.
>
> Maybe you could Cc NeilBrown <neilb@suse.com> for some more advice and
> I have no idea whether it's an expected behavior or not...
>
> Thanks,
> Gao Xiang
>
> On 2019/8/1 14:16, Sergei Turchanov wrote:
$ ./test /proc/meminfo 0 # Works as expected
MemTotal: 394907728 kB
MemFree: 173738328 kB
...
DirectMap2M: 13062144 kB
DirectMap1G: 390070272 kB
-----------------------------------------------------------------------
$ ./test /proc/meminfo 1024 # returns a copy of file after the remainder
Will seek to 1024
Data read at offset 1024
gePages: 0 kB
ShmemHugePages: 0 kB
ShmemPmdMapped: 0 kB
HugePages_Total: 0
HugePages_Free: 0
HugePages_Rsvd: 0
HugePages_Surp: 0
Hugepagesize: 2048 kB
Hugetlb: 0 kB
DirectMap4k: 245204 kB
DirectMap2M: 13062144 kB
DirectMap1G: 390070272 kB
MemTotal: 394907728 kB
MemFree: 173738328 kB
MemAvailable: 379989680 kB
Buffers: 355812 kB
Cached: 207216224 kB
...
DirectMap2M: 13062144 kB
DirectMap1G: 390070272 kB
As you see, after "DirectMap1G:" line, a whole copy of /proc/meminfo returned by "read".
Test program:
#include <sys/types.h>
#include <sys/stat.h>
#include <unistd.h>
#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>
#define SIZE 1024
char buf[SIZE + 1];
int main(int argc, char *argv[]) {
int fd;
ssize_t rd;
off_t ofs = 0;
if (argc < 2) {
printf("Usage: test <file> [<offset>]\n");
exit(1);
}
if (-1 == (fd = open(argv[1], O_RDONLY))) {
perror("open failed");
exit(1);
}
if (argc > 2) {
ofs = atol(argv[2]);
}
printf("Will seek to %ld\n", ofs);
if (-1 == (lseek(fd, ofs, SEEK_SET))) {
perror("lseek failed");
exit(1);
}
for (;; ofs += rd) {
printf("\n\nData read at offset %ld\n", ofs);
if (-1 == (rd = read(fd, buf, SIZE))) {
perror("read failed");
exit(1);
}
buf[rd] = '\0';
printf(buf);
if (rd < SIZE) {
break;
}
}
return 0;
}
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [BUG] lseek on /proc/meminfo is broken in 4.19.59 maybe due to commit 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and interface")
2019-08-01 8:09 ` [BUG] lseek on /proc/meminfo is broken in 4.19.59 maybe due to commit 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and interface") Sergei Turchanov
@ 2019-08-01 9:14 ` NeilBrown
2019-08-02 1:40 ` Sergei Turchanov
0 siblings, 1 reply; 11+ messages in thread
From: NeilBrown @ 2019-08-01 9:14 UTC (permalink / raw)
To: Sergei Turchanov; +Cc: linux-fsdevel, linux-kernel, Alexander Viro
[-- Attachment #1: Type: text/plain, Size: 5041 bytes --]
On Thu, Aug 01 2019, Sergei Turchanov wrote:
> Hello!
>
> [
> As suggested in previous discussion this behavior may be caused by your
> commit 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and interface")
> ]
Yes.... I think I can see what happened.
removing:
- if (!m->count) {
- m->from = 0;
- m->index++;
- }
from seq_read meant that ->index didn't get updated in a case that it
needs to be.
Please confirm that the following patch fixes the problem.
I think it is correct, but I need to look it over more carefully in the
morning, and see if I can explain why it is correct.
Thanks for the report.
NeilBrown
diff --git a/fs/seq_file.c b/fs/seq_file.c
index 04f09689cd6d..1600034a929b 100644
--- a/fs/seq_file.c
+++ b/fs/seq_file.c
@@ -119,6 +119,7 @@ static int traverse(struct seq_file *m, loff_t offset)
}
if (seq_has_overflowed(m))
goto Eoverflow;
+ p = m->op->next(m, p, &m->index);
if (pos + m->count > offset) {
m->from = offset - pos;
m->count -= m->from;
@@ -126,7 +127,6 @@ static int traverse(struct seq_file *m, loff_t offset)
}
pos += m->count;
m->count = 0;
- p = m->op->next(m, p, &m->index);
if (pos == offset)
break;
}
>
> Original bug report:
>
> Seeking (to an offset within file size) in /proc/meminfo is broken in 4.19.59. It does seek to a desired position, but reading from that position returns the remainder of file and then a whole copy of file. This doesn't happen with /proc/vmstat or /proc/self/maps for example.
>
> Seeking did work correctly in kernel 4.14.47. So it seems something broke in the way.
>
> Background: this kind of access pattern (seeking to /proc/meminfo) is used by libvirt-lxc fuse driver for virtualized view of /proc/meminfo. So that /proc/meminfo is broken in guests when running kernel 4.19.x.
>
> > On 01.08.2019 17:11, Gao Xiang wrote:
>> Hi,
>>
>> I just took a glance, maybe due to
>> commit 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and interface")
>>
>> I simply reverted it just now and it seems fine... but I haven't digged into this commit.
>>
>> Maybe you could Cc NeilBrown <neilb@suse.com> for some more advice and
>> I have no idea whether it's an expected behavior or not...
>>
>> Thanks,
>> Gao Xiang
>>
>> On 2019/8/1 14:16, Sergei Turchanov wrote:
>
>
> $ ./test /proc/meminfo 0 # Works as expected
>
> MemTotal: 394907728 kB
> MemFree: 173738328 kB
> ...
> DirectMap2M: 13062144 kB
> DirectMap1G: 390070272 kB
>
> -----------------------------------------------------------------------
>
> $ ./test /proc/meminfo 1024 # returns a copy of file after the remainder
>
> Will seek to 1024
>
>
> Data read at offset 1024
> gePages: 0 kB
> ShmemHugePages: 0 kB
> ShmemPmdMapped: 0 kB
> HugePages_Total: 0
> HugePages_Free: 0
> HugePages_Rsvd: 0
> HugePages_Surp: 0
> Hugepagesize: 2048 kB
> Hugetlb: 0 kB
> DirectMap4k: 245204 kB
> DirectMap2M: 13062144 kB
> DirectMap1G: 390070272 kB
> MemTotal: 394907728 kB
> MemFree: 173738328 kB
> MemAvailable: 379989680 kB
> Buffers: 355812 kB
> Cached: 207216224 kB
> ...
> DirectMap2M: 13062144 kB
> DirectMap1G: 390070272 kB
>
> As you see, after "DirectMap1G:" line, a whole copy of /proc/meminfo returned by "read".
>
> Test program:
>
> #include <sys/types.h>
> #include <sys/stat.h>
> #include <unistd.h>
> #include <fcntl.h>
> #include <stdio.h>
> #include <stdlib.h>
>
> #define SIZE 1024
> char buf[SIZE + 1];
>
> int main(int argc, char *argv[]) {
> int fd;
> ssize_t rd;
> off_t ofs = 0;
>
> if (argc < 2) {
> printf("Usage: test <file> [<offset>]\n");
> exit(1);
> }
>
> if (-1 == (fd = open(argv[1], O_RDONLY))) {
> perror("open failed");
> exit(1);
> }
>
> if (argc > 2) {
> ofs = atol(argv[2]);
> }
> printf("Will seek to %ld\n", ofs);
>
> if (-1 == (lseek(fd, ofs, SEEK_SET))) {
> perror("lseek failed");
> exit(1);
> }
>
> for (;; ofs += rd) {
> printf("\n\nData read at offset %ld\n", ofs);
> if (-1 == (rd = read(fd, buf, SIZE))) {
> perror("read failed");
> exit(1);
> }
> buf[rd] = '\0';
> printf(buf);
> if (rd < SIZE) {
> break;
> }
> }
>
> return 0;
> }
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [BUG] lseek on /proc/meminfo is broken in 4.19.59 maybe due to commit 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and interface")
2019-08-01 9:14 ` NeilBrown
@ 2019-08-02 1:40 ` Sergei Turchanov
2019-08-05 4:26 ` [PATCH] seq_file: fix problem when seeking mid-record NeilBrown
0 siblings, 1 reply; 11+ messages in thread
From: Sergei Turchanov @ 2019-08-02 1:40 UTC (permalink / raw)
To: NeilBrown; +Cc: linux-fsdevel, linux-kernel, Alexander Viro
Hello!
Yes, your patch fixed this bug.
Thank you very much!
With best regards,
Sergei.
On 01.08.2019 19:14, NeilBrown wrote:
> On Thu, Aug 01 2019, Sergei Turchanov wrote:
>
>> Hello!
>>
>> [
>> As suggested in previous discussion this behavior may be caused by your
>> commit 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and interface")
>> ]
> Yes.... I think I can see what happened.
> removing:
> - if (!m->count) {
> - m->from = 0;
> - m->index++;
> - }
>
> from seq_read meant that ->index didn't get updated in a case that it
> needs to be.
>
> Please confirm that the following patch fixes the problem.
> I think it is correct, but I need to look it over more carefully in the
> morning, and see if I can explain why it is correct.
>
> Thanks for the report.
> NeilBrown
>
> diff --git a/fs/seq_file.c b/fs/seq_file.c
> index 04f09689cd6d..1600034a929b 100644
> --- a/fs/seq_file.c
> +++ b/fs/seq_file.c
> @@ -119,6 +119,7 @@ static int traverse(struct seq_file *m, loff_t offset)
> }
> if (seq_has_overflowed(m))
> goto Eoverflow;
> + p = m->op->next(m, p, &m->index);
> if (pos + m->count > offset) {
> m->from = offset - pos;
> m->count -= m->from;
> @@ -126,7 +127,6 @@ static int traverse(struct seq_file *m, loff_t offset)
> }
> pos += m->count;
> m->count = 0;
> - p = m->op->next(m, p, &m->index);
> if (pos == offset)
> break;
> }
>
>
>> Original bug report:
>>
>> Seeking (to an offset within file size) in /proc/meminfo is broken in 4.19.59. It does seek to a desired position, but reading from that position returns the remainder of file and then a whole copy of file. This doesn't happen with /proc/vmstat or /proc/self/maps for example.
>>
>> Seeking did work correctly in kernel 4.14.47. So it seems something broke in the way.
>>
>> Background: this kind of access pattern (seeking to /proc/meminfo) is used by libvirt-lxc fuse driver for virtualized view of /proc/meminfo. So that /proc/meminfo is broken in guests when running kernel 4.19.x.
>>
>> > On 01.08.2019 17:11, Gao Xiang wrote:
>>> Hi,
>>>
>>> I just took a glance, maybe due to
>>> commit 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and interface")
>>>
>>> I simply reverted it just now and it seems fine... but I haven't digged into this commit.
>>>
>>> Maybe you could Cc NeilBrown <neilb@suse.com> for some more advice and
>>> I have no idea whether it's an expected behavior or not...
>>>
>>> Thanks,
>>> Gao Xiang
>>>
>>> On 2019/8/1 14:16, Sergei Turchanov wrote:
>>
>> $ ./test /proc/meminfo 0 # Works as expected
>>
>> MemTotal: 394907728 kB
>> MemFree: 173738328 kB
>> ...
>> DirectMap2M: 13062144 kB
>> DirectMap1G: 390070272 kB
>>
>> -----------------------------------------------------------------------
>>
>> $ ./test /proc/meminfo 1024 # returns a copy of file after the remainder
>>
>> Will seek to 1024
>>
>>
>> Data read at offset 1024
>> gePages: 0 kB
>> ShmemHugePages: 0 kB
>> ShmemPmdMapped: 0 kB
>> HugePages_Total: 0
>> HugePages_Free: 0
>> HugePages_Rsvd: 0
>> HugePages_Surp: 0
>> Hugepagesize: 2048 kB
>> Hugetlb: 0 kB
>> DirectMap4k: 245204 kB
>> DirectMap2M: 13062144 kB
>> DirectMap1G: 390070272 kB
>> MemTotal: 394907728 kB
>> MemFree: 173738328 kB
>> MemAvailable: 379989680 kB
>> Buffers: 355812 kB
>> Cached: 207216224 kB
>> ...
>> DirectMap2M: 13062144 kB
>> DirectMap1G: 390070272 kB
>>
>> As you see, after "DirectMap1G:" line, a whole copy of /proc/meminfo returned by "read".
>>
>> Test program:
>>
>> #include <sys/types.h>
>> #include <sys/stat.h>
>> #include <unistd.h>
>> #include <fcntl.h>
>> #include <stdio.h>
>> #include <stdlib.h>
>>
>> #define SIZE 1024
>> char buf[SIZE + 1];
>>
>> int main(int argc, char *argv[]) {
>> int fd;
>> ssize_t rd;
>> off_t ofs = 0;
>>
>> if (argc < 2) {
>> printf("Usage: test <file> [<offset>]\n");
>> exit(1);
>> }
>>
>> if (-1 == (fd = open(argv[1], O_RDONLY))) {
>> perror("open failed");
>> exit(1);
>> }
>>
>> if (argc > 2) {
>> ofs = atol(argv[2]);
>> }
>> printf("Will seek to %ld\n", ofs);
>>
>> if (-1 == (lseek(fd, ofs, SEEK_SET))) {
>> perror("lseek failed");
>> exit(1);
>> }
>>
>> for (;; ofs += rd) {
>> printf("\n\nData read at offset %ld\n", ofs);
>> if (-1 == (rd = read(fd, buf, SIZE))) {
>> perror("read failed");
>> exit(1);
>> }
>> buf[rd] = '\0';
>> printf(buf);
>> if (rd < SIZE) {
>> break;
>> }
>> }
>>
>> return 0;
>> }
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH] seq_file: fix problem when seeking mid-record.
2019-08-02 1:40 ` Sergei Turchanov
@ 2019-08-05 4:26 ` NeilBrown
2019-08-05 9:15 ` Markus Elfring
2019-08-06 22:42 ` [PATCH] " Andrew Morton
0 siblings, 2 replies; 11+ messages in thread
From: NeilBrown @ 2019-08-05 4:26 UTC (permalink / raw)
To: Andrew Morton, Alexander Viro, Sergei Turchanov
Cc: linux-fsdevel, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1817 bytes --]
If you use lseek or similar (e.g. pread) to access
a location in a seq_file file that is within a record,
rather than at a record boundary, then the first read
will return the remainder of the record, and the second
read will return the whole of that same record (instead
of the next record).
Whnn seeking to a record boundary, the next record is
correctly returned.
This bug was introduced by a recent patch (identified below)
Before that patch, seq_read() would increment m->index when
the last of the buffer was returned (m->count == 0).
After that patch, we rely on ->next to increment m->index
after filling the buffer - but there was one place where that
didn't happen.
Link: https://lkml.kernel.org/lkml/877e7xl029.fsf@notabene.neil.brown.name/
Reported-by-tested-by: Sergei Turchanov <turchanov@farpost.com>
Fixes: 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code
and interface")
Cc: stable@vger.kernel.org (v4.19+)
Signed-off-by: NeilBrown <neilb@suse.com>
---
Hi Andrew: as you applied the offending patch for me, maybe you could
queue up this fix too.
Thanks,
NeilBrown
fs/seq_file.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/seq_file.c b/fs/seq_file.c
index 04f09689cd6d..1600034a929b 100644
--- a/fs/seq_file.c
+++ b/fs/seq_file.c
@@ -119,6 +119,7 @@ static int traverse(struct seq_file *m, loff_t offset)
}
if (seq_has_overflowed(m))
goto Eoverflow;
+ p = m->op->next(m, p, &m->index);
if (pos + m->count > offset) {
m->from = offset - pos;
m->count -= m->from;
@@ -126,7 +127,6 @@ static int traverse(struct seq_file *m, loff_t offset)
}
pos += m->count;
m->count = 0;
- p = m->op->next(m, p, &m->index);
if (pos == offset)
break;
}
--
2.14.0.rc0.dirty
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: seq_file: fix problem when seeking mid-record.
2019-08-05 4:26 ` [PATCH] seq_file: fix problem when seeking mid-record NeilBrown
@ 2019-08-05 9:15 ` Markus Elfring
2019-08-05 10:08 ` NeilBrown
2019-08-06 22:42 ` [PATCH] " Andrew Morton
1 sibling, 1 reply; 11+ messages in thread
From: Markus Elfring @ 2019-08-05 9:15 UTC (permalink / raw)
To: Neil Brown, Andrew Morton, Alexander Viro
Cc: linux-fsdevel, linux-kernel, Sergei Turchanov
> Fixes: 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code
> and interface")
Please do not split this tag across multiple lines in the final commit description.
Regards,
Markus
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: seq_file: fix problem when seeking mid-record.
2019-08-05 9:15 ` Markus Elfring
@ 2019-08-05 10:08 ` NeilBrown
0 siblings, 0 replies; 11+ messages in thread
From: NeilBrown @ 2019-08-05 10:08 UTC (permalink / raw)
To: Markus Elfring, Andrew Morton, Alexander Viro
Cc: Sergei Turchanov, linux-fsdevel, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 547 bytes --]
On Mon, Aug 05 2019, Markus Elfring wrote:
>> Fixes: 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code
>> and interface")
>
> Please do not split this tag across multiple lines in the final commit description.
I tend to agree...
I had previously seen
"Possible unwrapped commit description (prefer a maximum 75 chars per line)\n"
warnings from checkpatch, but one closer look that doesn't apply to
Fixes: lines (among other special cases).
Maybe Andrew will fix it up for me when it applies .... (please!)
Thanks,
NeilBrown
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] seq_file: fix problem when seeking mid-record.
2019-08-05 4:26 ` [PATCH] seq_file: fix problem when seeking mid-record NeilBrown
2019-08-05 9:15 ` Markus Elfring
@ 2019-08-06 22:42 ` Andrew Morton
1 sibling, 0 replies; 11+ messages in thread
From: Andrew Morton @ 2019-08-06 22:42 UTC (permalink / raw)
To: NeilBrown; +Cc: Alexander Viro, Sergei Turchanov, linux-fsdevel, linux-kernel
On Mon, 05 Aug 2019 14:26:08 +1000 NeilBrown <neilb@suse.com> wrote:
> If you use lseek or similar (e.g. pread) to access
> a location in a seq_file file that is within a record,
> rather than at a record boundary, then the first read
> will return the remainder of the record, and the second
> read will return the whole of that same record (instead
> of the next record).
> Whnn seeking to a record boundary, the next record is
> correctly returned.
ouch. I'm surprised it took this long to be noticed.
Maybe we need a seqfile-basher in tools/testing/selftests/proc or
somewhere.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [BUG] lseek on /proc/meminfo is broken in 4.19.59
@ 2019-07-18 3:26 Sergei Turchanov
0 siblings, 0 replies; 11+ messages in thread
From: Sergei Turchanov @ 2019-07-18 3:26 UTC (permalink / raw)
To: Alexander Viro, linux-fsdevel; +Cc: linux-kernel
Hello!
Seeking (to an offset within file size) in /proc/meminfo is broken in
4.19.59. It does seek to a desired position, but reading from that
position returns the remainder of file and then a whole copy of file.
This doesn't happen with /proc/vmstat or /proc/self/maps for example.
Seeking did work correctly in kernel 4.14.47. So it seems something
broke in the way.
Background: this kind of access pattern (seeking to /proc/meminfo) is
used by libvirt-lxc fuse driver for virtualized view of /proc/meminfo.
$ ./test /proc/meminfo 0 # Works as expected
MemTotal: 394907728 kB
MemFree: 173738328 kB
...
DirectMap2M: 13062144 kB
DirectMap1G: 390070272 kB
-----------------------------------------------------------------------
$ ./test 1024 # returns a copy of file after the remainder
Will seek to 1024
Data read at offset 1024
gePages: 0 kB
ShmemHugePages: 0 kB
ShmemPmdMapped: 0 kB
HugePages_Total: 0
HugePages_Free: 0
HugePages_Rsvd: 0
HugePages_Surp: 0
Hugepagesize: 2048 kB
Hugetlb: 0 kB
DirectMap4k: 245204 kB
DirectMap2M: 13062144 kB
DirectMap1G: 390070272 kB
MemTotal: 394907728 kB
MemFree: 173738328 kB
MemAvailable: 379989680 kB
Buffers: 355812 kB
Cached: 207216224 kB
...
DirectMap2M: 13062144 kB
DirectMap1G: 390070272 kB
As you see, after "DirectMap1G:" line, a whole copy of /proc/meminfo
returned by "read".
Test program:
#include <sys/types.h>
#include <sys/stat.h>
#include <unistd.h>
#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>
#define SIZE 1024
char buf[SIZE + 1];
int main(int argc, char *argv[]) {
int fd;
ssize_t rd;
off_t ofs = 0;
if (argc < 2) {
printf("Usage: test <file> [<offset>]\n");
exit(1);
}
if (-1 == (fd = open(argv[1], O_RDONLY))) {
perror("open failed");
exit(1);
}
if (argc > 2) {
ofs = atol(argv[2]);
}
printf("Will seek to %ld\n", ofs);
if (-1 == (lseek(fd, ofs, SEEK_SET))) {
perror("lseek failed");
exit(1);
}
for (;; ofs += rd) {
printf("\n\nData read at offset %ld\n", ofs);
if (-1 == (rd = read(fd, buf, SIZE))) {
perror("read failed");
exit(1);
}
buf[rd] = '\0';
printf(buf);
if (rd < SIZE) {
break;
}
}
return 0;
}
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-08-06 22:42 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-01 6:16 [BUG] lseek on /proc/meminfo is broken in 4.19.59 Sergei Turchanov
2019-08-01 7:11 ` Gao Xiang
2019-08-01 7:52 ` Sergei Turchanov
2019-08-01 8:09 ` [BUG] lseek on /proc/meminfo is broken in 4.19.59 maybe due to commit 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and interface") Sergei Turchanov
2019-08-01 9:14 ` NeilBrown
2019-08-02 1:40 ` Sergei Turchanov
2019-08-05 4:26 ` [PATCH] seq_file: fix problem when seeking mid-record NeilBrown
2019-08-05 9:15 ` Markus Elfring
2019-08-05 10:08 ` NeilBrown
2019-08-06 22:42 ` [PATCH] " Andrew Morton
-- strict thread matches above, loose matches on Subject: below --
2019-07-18 3:26 [BUG] lseek on /proc/meminfo is broken in 4.19.59 Sergei Turchanov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).