* [PATCH] seq_file: don't set read position for invalid iterator
@ 2016-08-23 9:19 Tomasz Majchrzak
2016-09-23 15:54 ` Dan Williams
0 siblings, 1 reply; 2+ messages in thread
From: Tomasz Majchrzak @ 2016-08-23 9:19 UTC (permalink / raw)
To: linux-fsdevel
Cc: aleksey.obitotskiy, pawel.baldysiak, artur.paszkiewicz, maksymilian.kunt
If kernfs file is empty on a first read, successive read operations
using the same file descriptor will return no data, even when data is
available. Default kernfs 'seq_next' implementation advances iterator
position even when next object is not there. Kernfs 'seq_start' for
following requests will not return iterator as position is already on
the second object.
Don't set read position if valid iterator has not been returned.
Signed-off-by: Tomasz Majchrzak <tomasz.majchrzak@intel.com>
---
fs/seq_file.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/seq_file.c b/fs/seq_file.c
index 19f532e..893db43 100644
--- a/fs/seq_file.c
+++ b/fs/seq_file.c
@@ -242,7 +242,8 @@ ssize_t seq_read(struct file *file, char __user *buf, size_t size, loff_t *ppos)
m->count = 0;
if (unlikely(!m->count)) {
p = m->op->next(m, p, &pos);
- m->index = pos;
+ if (p && !IS_ERR(p))
+ m->index = pos;
continue;
}
if (m->count < m->size)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] seq_file: don't set read position for invalid iterator
2016-08-23 9:19 [PATCH] seq_file: don't set read position for invalid iterator Tomasz Majchrzak
@ 2016-09-23 15:54 ` Dan Williams
0 siblings, 0 replies; 2+ messages in thread
From: Dan Williams @ 2016-09-23 15:54 UTC (permalink / raw)
To: Tomasz Majchrzak
Cc: linux-fsdevel, aleksey.obitotskiy, Baldysiak, Pawel,
Artur Paszkiewicz, maksymilian.kunt, Al Viro,
Linux Kernel Mailing List
[ adding Al ]
On Tue, Aug 23, 2016 at 2:19 AM, Tomasz Majchrzak
<tomasz.majchrzak@intel.com> wrote:
> If kernfs file is empty on a first read, successive read operations
> using the same file descriptor will return no data, even when data is
> available. Default kernfs 'seq_next' implementation advances iterator
> position even when next object is not there. Kernfs 'seq_start' for
> following requests will not return iterator as position is already on
> the second object.
>
> Don't set read position if valid iterator has not been returned.
Can you say a bit about how you found this? So we have an idea about
userspace tooling is broken by this bug.
>
> Signed-off-by: Tomasz Majchrzak <tomasz.majchrzak@intel.com>
I would add "Cc: <stable@vger.kernel.org>" to the next version of this patch.
> fs/seq_file.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/seq_file.c b/fs/seq_file.c
> index 19f532e..893db43 100644
> --- a/fs/seq_file.c
> +++ b/fs/seq_file.c
> @@ -242,7 +242,8 @@ ssize_t seq_read(struct file *file, char __user *buf, size_t size, loff_t *ppos)
> m->count = 0;
> if (unlikely(!m->count)) {
> p = m->op->next(m, p, &pos);
> - m->index = pos;
> + if (p && !IS_ERR(p))
Lets change this to use the IS_ERR_OR_NULL() helper, and do the same
with the earlier if (!p || IS_ERR()) a few lines up in the loop.
Other than that, this looks good to me.
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2016-09-23 15:54 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-23 9:19 [PATCH] seq_file: don't set read position for invalid iterator Tomasz Majchrzak
2016-09-23 15:54 ` Dan Williams
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).