2.2.14: fbcon: Oops on fbset -a - code is obviously wrong

From: Russell King (rmk@arm.linux.org.uk)
Date: Sat Apr 22 2000 - 13:21:11 EDT

  • Next message: kuznet@ms2.inr.ac.ru: "Re: [PATCH] f_op->poll() without lock_kernel()"

    Hi,

    While investigating why someone has received an oops on their 2.2.14 setup,
    I've come across the following code in drivers/char/console.c:

            unsigned short *newscreens[MAX_NR_CONSOLES];

            for (currcons = first; currcons <= last; currcons++) {
                    if (!vc_cons_allocated(currcons) ||
                        (cc == video_num_columns && ll == video_num_lines))
                            newscreens[currcons] = NULL;
                    else {
                            unsigned short *p = (unsigned short *) kmalloc(ss, GFP_USER);
                            if (!p) {
                                    for (i = 0; i< currcons; i++)
                                            if (newscreens[i])
                                                    kfree_s(newscreens[i], ss);
                                    return -ENOMEM;
                            }
                            newscreens[currcons] = p;
                            todo++;
                    }
            }

    We start at console "first", allocating kmalloc'd buffers (which may be big,
    and therefore likely to fail). When we fail, we start freeing the buffers
    starting at console "0", not "first".

    Surely this can not be right, especially as "newscreens" is allocated on the
    stack!

    However, I've not checked the above in Alan's latest incarnation of 2.2.15pre.
    If it hasn't been fixed, I suggest that the following goes into 2.2.15pre ASAP:

    --- /usr/src/v2.2/linux/drivers/char/console.c.orig Sat Apr 22 18:19:49 2000
    +++ /usr/src/v2.2/linux/drivers/char/console.c Sat Apr 22 18:20:00 2000
    @@ -681,7 +681,7 @@
                     else {
                             unsigned short *p = (unsigned short *) kmalloc(ss, GFP_USER);
                             if (!p) {
    - for (i = 0; i< currcons; i++)
    + for (i = first; i< currcons; i++)
                                             if (newscreens[i])
                                                     kfree_s(newscreens[i], ss);
                                     return -ENOMEM;

       _____
      |_____| ------------------------------------------------- ---+---+-
      | | Russell King rmk@arm.linux.org.uk --- ---
      | | | | http://www.arm.linux.org.uk/~rmk/aboutme.html / / |
      | +-+-+ --- -+-
      / | THE developer of ARM Linux |+| /|\
     / | | | --- |
        +-+-+ ------------------------------------------------- /\\\ |

    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.rutgers.edu
    Please read the FAQ at http://www.tux.org/lkml/



    This archive was generated by hypermail 2b29 : Sat Apr 22 2000 - 13:44:15 EDT