lkml.org 
[lkml]   [2007]   [Aug]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [patch 2/2] Sort module list by pointer address to get coherent sleepable seq_file iterators
    * Rusty Russell (rusty@rustcorp.com.au) wrote:
    > On Mon, 2007-08-20 at 16:26 -0400, Mathieu Desnoyers wrote:
    > > plain text document attachment (module.c-sort-module-list.patch)
    > > A race that appears both in /proc/modules and in kallsyms: if, between the
    > > seq file reads, the process is put to sleep and at this moment a module is
    > > or removed from the module list, the listing will skip an amount of
    > > modules/symbols corresponding to the amount of elements present in the unloaded
    > > module, but at the current position in the list if the iteration is located
    > > after the removed module.
    > >
    > > The cleanest way I found to deal with this problem is to sort the module list.
    > > We can then keep the old struct module * as the old iterator, knowing the it may
    > > be removed between the seq file reads, but we only use it as "get next". If it
    > > is not present in the module list, the next pointer will be used.
    > >
    > > By doing this, removing a given module will now only fuzz the output related to
    > > this specific module, not any random module anymore. Since modprobe uses
    > > /proc/modules, it might be important to make sure multiple concurrent running
    > > modprobes won't interfere with each other.
    >
    > You've reduced, but not eliminated, the problem. A new module inserted
    > is quite likely to reuse the same address.
    >

    Hi Rusty,

    Please tell me if I'm wrong, but I think it would not be a problem:

    - seq_read() makes sure that a buffer large enough is available so that
    m_show() can fully extract and print the information relative to 1
    module.
    - m_start() and m_stop() takes the module_mutex, therefore within one
    seq_read(), once m_start has returned, the struct module * that we
    have is valid and will be consistent during the whole seq_read
    operation.
    - If a module is removed, and then a different one is inserted at the
    same address, while we are between two seq_reads for this given struct
    module address, the seq_reads will copy to user-space the information
    that is still in the buffer for the _first_ struct module encountered,
    not the new one.
    - After that, iteration will continue to the new struct module address,
    effectively skipping the newly inserted module.


    > I don't have a real problem with this patch, but I'm wondering if the
    > problem is theoretical or demonstrated.
    >

    Small test module for this:

    #include <stdio.h>
    #include <sys/types.h>
    #include <sys/stat.h>
    #include <fcntl.h>

    #define BUFSIZE 1024

    int main()
    {
    int fd = open("/proc/modules", O_RDONLY);
    char buf[BUFSIZE];
    ssize_t size;

    do {
    size = read(fd, buf, 1);
    printf("%c", buf[0]);
    usleep(100000);
    } while(size > 0);

    close(fd);
    return 0;
    }

    (compiled as ./module)

    Actual test (kernel 2.6.23-rc3):

    dijkstra:~# lsmod
    Module Size Used by
    pl2303 18564 0
    usbserial 29032 1 pl2303
    ppdev 7844 0
    sky2 37476 0
    skge 36368 0
    rtc 10104 0
    snd_hda_intel 265628 0

    (here, while we are printing the 2nd line, I rmmod pl2303)
    compudj@dijkstra:~/test$ ./module
    pl2303 18564 0 - Live 0xf886e000
    usbserial 29032 1 pl2303, Live 0xf8865000
    sky2 37476 0 - Live 0xf884f000
    skge 36368 0 - Live 0xf8838000
    rtc 10104 0 - Live 0xf8825000

    We see the the 2nd line is garbage.

    Now, with my patch applied:
    (here, while we are printing the rtc module, I rmmod rtc)
    nd_hda_intel 268708 0 - Live 0xf8820000
    ltt_control 2372 0 - Live 0xf8866000
    rtc 10392 0 - Live 0xf886d000
    skge 36768 0 - Live 0xf8871000
    ltt_statedump 8516 0 - Live 0xf887b000

    We see that since the rtc line was already in the buffer, it has been
    printed completely.


    (here, while we are printing the skge module, I rmmod rtc)
    snd_hda_intel 268708 0 - Live 0xf8820000
    ltt_control 2372 0 - Live 0xf8866000
    rtc 10392 0 - Live 0xf886d000
    skge 36768 0 - Live 0xf8871000
    ltt_statedump 8516 0 - Live 0xf887b000
    sky2 38420 0 - Live 0xf88cd000

    We see that the iteration continued at the same position even though the
    rtc module, located in earlier addresses, was removed.

    Note that this test is done with the
    "Sort modules list - use ppos instead of m->private"
    patch applied.

    Thanks for the review,

    Mathieu

    > Rusty.
    >
    >

    --
    Mathieu Desnoyers
    Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
    OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2007-08-24 17:49    [W:3.893 / U:0.388 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site