lkml.org 
[lkml]   [2012]   [Jan]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 1/1] Fix seq_file mishandling of consecutive pread() invocations.
    [ Added Maintainers; Added reference to bugzilla.kernel.org in commit log ]

    Also reported in:

    https://bugzilla.kernel.org/show_bug.cgi?id=11856

    The following program illustrates the problem:

    char buf[8192];

    int fd = open("/proc/self/maps", O_RDONLY);

    n = pread(fd, buf, sizeof(buf), 0);
    printf("%d\n", n);

    /* lseek(fd, 0, SEEK_CUR); */ /* Uncomment to work around */

    n = pread(fd, buf, sizeof(buf), 0);
    printf("%d\n", n);

    The second printf() prints zero, but uncommenting the lseek()
    corrects its behaviour.

    To fix, make seq_read() mirror seq_lseek() when processing changes in *ppos.
    Restore m->version first, then if required traverse and update read_pos
    on success.

    Signed-off-by: Earl Chew <echew@ixiacom.com>
    ---
    fs/seq_file.c | 28 +++++++++++++++-------------
    1 files changed, 15 insertions(+), 13 deletions(-)

    diff --git a/fs/seq_file.c b/fs/seq_file.c
    index dba43c3..7a45306 100644
    --- a/fs/seq_file.c
    +++ b/fs/seq_file.c
    @@ -140,9 +140,21 @@ ssize_t seq_read(struct file *file, char __user *buf, size_t size, loff_t *ppos)

    mutex_lock(&m->lock);

    + /*
    + * seq_file->op->..m_start/m_stop/m_next may do special actions
    + * or optimisations based on the file->f_version, so we want to
    + * pass the file->f_version to those methods.
    + *
    + * seq_file->version is just copy of f_version, and seq_file
    + * methods can treat it simply as file version.
    + * It is copied in first and copied out after all operations.
    + * It is convenient to have it as part of structure to avoid the
    + * need of passing another argument to all the seq_file methods.
    + */
    + m->version = file->f_version;
    +
    /* Don't assume *ppos is where we left it */
    if (unlikely(*ppos != m->read_pos)) {
    - m->read_pos = *ppos;
    while ((err = traverse(m, *ppos)) == -EAGAIN)
    ;
    if (err) {
    @@ -152,21 +164,11 @@ ssize_t seq_read(struct file *file, char __user *buf, size_t size, loff_t *ppos)
    m->index = 0;
    m->count = 0;
    goto Done;
    + } else {
    + m->read_pos = *ppos;
    }
    }

    - /*
    - * seq_file->op->..m_start/m_stop/m_next may do special actions
    - * or optimisations based on the file->f_version, so we want to
    - * pass the file->f_version to those methods.
    - *
    - * seq_file->version is just copy of f_version, and seq_file
    - * methods can treat it simply as file version.
    - * It is copied in first and copied out after all operations.
    - * It is convenient to have it as part of structure to avoid the
    - * need of passing another argument to all the seq_file methods.
    - */
    - m->version = file->f_version;
    /* grab buffer if we didn't have one */
    if (!m->buf) {
    m->buf = kmalloc(m->size = PAGE_SIZE, GFP_KERNEL);
    --
    1.7.0.4



    \
     
     \ /
      Last update: 2012-01-22 20:03    [W:0.026 / U:0.492 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site