[PATCH] Fix crash in viafb due to 4k stack overflow - Kernel

This is a discussion on [PATCH] Fix crash in viafb due to 4k stack overflow - Kernel ; The function viafb_cursor() uses 2 stack-variables of CURSOR_SIZE bits; CURSOR_SIZE is defined as (8 * 1024). Using up twice 1k on stack is too much for 4k-stack (though it works with 8k-stacks). Make those two variables kzalloc'ed to preserve stack ...

+ Reply to Thread
Results 1 to 9 of 9

Thread: [PATCH] Fix crash in viafb due to 4k stack overflow

  1. [PATCH] Fix crash in viafb due to 4k stack overflow

    The function viafb_cursor() uses 2 stack-variables of CURSOR_SIZE bits;
    CURSOR_SIZE is defined as (8 * 1024). Using up twice 1k on stack is too much
    for 4k-stack (though it works with 8k-stacks).

    Make those two variables kzalloc'ed to preserve stack space.

    Signed-off-by: Bruno Prémont

    ---
    --- linux-2.6.28-rc3.orig/drviers/video/via/viafbdev.c 2008-11-09 19:22:15.000000000 +0100
    +++ linux-2.6.28-rc3/drivers/video/via/viafbdev.c 2008-11-09 19:36:15.000000000 +0100
    @@ -1052,10 +1052,8 @@ static void viafb_imageblit(struct fb_in

    static int viafb_cursor(struct fb_info *info, struct fb_cursor *cursor)
    {
    - u8 data[CURSOR_SIZE / 8];
    - u32 data_bak[CURSOR_SIZE / 32];
    u32 temp, xx, yy, bg_col = 0, fg_col = 0;
    - int size, i, j = 0;
    + int i, j = 0;
    static int hw_cursor;
    struct viafb_par *p_viafb_par;

    @@ -1178,10 +1176,15 @@ static int viafb_cursor(struct fb_info *
    }

    if (cursor->set & FB_CUR_SETSHAPE) {
    - size =
    + u8 *data = kzalloc(CURSOR_SIZE / 8, GFP_KERNEL);
    + u32 *data_bak = kzalloc(CURSOR_SIZE / 32, GFP_KERNEL);
    + int size =
    ((viacursor.image.width + 7) >> 3) *
    viacursor.image.height;

    + if (data == NULL || data_bak == NULL)
    + goto out;
    +
    if (MAX_CURS == 32) {
    for (i = 0; i < (CURSOR_SIZE / 32); i++) {
    data_bak[i] = 0x0;
    @@ -1231,6 +1234,9 @@ static int viafb_cursor(struct fb_info *
    memcpy(((struct viafb_par *)(info->par))->fbmem_virt +
    ((struct viafb_par *)(info->par))->cursor_start,
    data_bak, CURSOR_SIZE);
    +out:
    + kfree(data);
    + kfree(data_bak);
    }

    if (viacursor.enable)
    --
    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/

  2. Re: [PATCH] Fix crash in viafb due to 4k stack overflow

    On Sun, 9 Nov 2008 20:25:37 +0100 Bruno Pr__mont wrote:

    > The function viafb_cursor() uses 2 stack-variables of CURSOR_SIZE bits;
    > CURSOR_SIZE is defined as (8 * 1024). Using up twice 1k on stack is too much
    > for 4k-stack (though it works with 8k-stacks).


    yup, we should fix that.

    > Make those two variables kzalloc'ed to preserve stack space.
    >
    > Signed-off-by: Bruno Pr__mont
    >
    > ---
    > --- linux-2.6.28-rc3.orig/drviers/video/via/viafbdev.c 2008-11-09 19:22:15.000000000 +0100


    ^^ typo in pathname?

    > +++ linux-2.6.28-rc3/drivers/video/via/viafbdev.c 2008-11-09 19:36:15.000000000 +0100
    > @@ -1052,10 +1052,8 @@ static void viafb_imageblit(struct fb_in
    >
    > static int viafb_cursor(struct fb_info *info, struct fb_cursor *cursor)
    > {
    > - u8 data[CURSOR_SIZE / 8];
    > - u32 data_bak[CURSOR_SIZE / 32];
    > u32 temp, xx, yy, bg_col = 0, fg_col = 0;
    > - int size, i, j = 0;
    > + int i, j = 0;
    > static int hw_cursor;
    > struct viafb_par *p_viafb_par;
    >
    > @@ -1178,10 +1176,15 @@ static int viafb_cursor(struct fb_info *
    > }
    >
    > if (cursor->set & FB_CUR_SETSHAPE) {
    > - size =
    > + u8 *data = kzalloc(CURSOR_SIZE / 8, GFP_KERNEL);
    > + u32 *data_bak = kzalloc(CURSOR_SIZE / 32, GFP_KERNEL);
    > + int size =
    > ((viacursor.image.width + 7) >> 3) *
    > viacursor.image.height;
    >
    > + if (data == NULL || data_bak == NULL)
    > + goto out;
    > +
    > if (MAX_CURS == 32) {
    > for (i = 0; i < (CURSOR_SIZE / 32); i++) {
    > data_bak[i] = 0x0;
    > @@ -1231,6 +1234,9 @@ static int viafb_cursor(struct fb_info *
    > memcpy(((struct viafb_par *)(info->par))->fbmem_virt +
    > ((struct viafb_par *)(info->par))->cursor_start,
    > data_bak, CURSOR_SIZE);
    > +out:
    > + kfree(data);
    > + kfree(data_bak);
    > }
    >
    > if (viacursor.enable)


    Is the ->fb_cursor handler allowed to perform GFP_KERNEL memory
    allocations? It's never called from atomic contexts?

    --
    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/

  3. Re: [PATCH] Fix crash in viafb due to 4k stack overflow

    On Sun, 9 Nov 2008 11:36:03 -0800
    Andrew Morton wrote:

    > On Sun, 9 Nov 2008 20:25:37 +0100 Bruno Pr__mont
    > wrote:
    >
    > > The function viafb_cursor() uses 2 stack-variables of CURSOR_SIZE
    > > bits; CURSOR_SIZE is defined as (8 * 1024). Using up twice 1k on
    > > stack is too much for 4k-stack (though it works with 8k-stacks).

    >
    > >
    > > if (viacursor.enable)

    >
    > Is the ->fb_cursor handler allowed to perform GFP_KERNEL memory
    > allocations? It's never called from atomic contexts?


    if it's allowed to do GFP_KERNEL memory allocations the statement that
    it works with 8k stacks is a bit overstated... since irq's can come in
    and take several KB of stack as well

    --
    Arjan van de Ven Intel Open Source Technology Centre
    For development, discussion and tips for power savings,
    visit http://www.lesswatts.org
    --
    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/

  4. Re: [PATCH] Fix crash in viafb due to 4k stack overflow

    On Sun, 09 November 2008 Arjan van de Ven wrote:
    > On Sun, 9 Nov 2008 Andrew Morton wrote:
    > > On Sun, 9 Nov 2008 Bruno Prémont wrote:
    > >
    > > > The function viafb_cursor() uses 2 stack-variables of CURSOR_SIZE
    > > > bits; CURSOR_SIZE is defined as (8 * 1024). Using up twice 1k on
    > > > stack is too much for 4k-stack (though it works with 8k-stacks).

    > >
    > > >
    > > > if (viacursor.enable)

    > >
    > > Is the ->fb_cursor handler allowed to perform GFP_KERNEL memory
    > > allocations? It's never called from atomic contexts?

    >
    > if it's allowed to do GFP_KERNEL memory allocations the statement that
    > it works with 8k stacks is a bit overstated... since irq's can come in
    > and take several KB of stack as well

    I don't know if it can be called from atomic contexts or not


    In addition I get panics some time after start-up which I'm not sure
    what to associate them with (apparently unrelated)
    It could be some stack overflow by calling fbset (I will to more testing
    in order to find out)

    First attempt: calling fbset via ssh:

    [ 1806.952151] BUG: unable to handle kernel NULL pointer dereference at 00000123
    [ 1806.952601] IP: [] icmpv6_send+0x387/0x580
    [ 1806.952934] *pde = 00000000
    [ 1806.953125] Oops: 0000 [#1]
    [ 1806.953310] last sysfs file: /sys/devices/platform/w83627hf.656/temp2_input
    [ 1806.953717] Modules linked in: snd_hda_intel snd_pcm snd_timer snd soundcore snd_page_alloc sg
    [ 1806.954328]
    [ 1806.954430] Pid: 1855, comm: sshd Not tainted (2.6.28-rc3-git6 #1) CX700+W697HG
    [ 1806.954863] EIP: 0060:[] EFLAGS: 00010206 CPU: 0
    [ 1806.955194] EIP is at icmpv6_send+0x387/0x580
    [ 1806.955456] EAX: ffffffff EBX: f713c704 ECX: f6bc26a8 EDX: 0000006c
    [ 1806.955827] ESI: f713c500 EDI: 00000040 EBP: f6babca0 ESP: f6babbf8
    [ 1806.956197] DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 0068
    [ 1806.956520] Process sshd (pid: 1855, ti=f6bab000 task=f70e7440 task.ti=f6bab000)
    [ 1806.956952] Stack:
    [ 1806.957074] 00000000 00007515 ffffffcc f6babc51 f6babc51 c05f6045 f6babc8c 00000296
    [ 1806.957614] f6babc3c 00000000 00000002 f6bc26a8 00000200 38b782ca f713c704 00000000
    [ 1806.958321] 00000000 00000000 00000000 00000000 60090120 0000ab07 ff1d0302 300005fe
    [ 1806.958882] Call Trace:
    [ 1806.959037] [] ? ip6_xmit+0x230/0x3f0
    [ 1806.959339] [] ? inet6_csk_xmit+0x103/0x190
    [ 1806.959669] [] ? tcp_v6_send_check+0x51/0x100
    [ 1806.960011] [] ? tcp_transmit_skb+0x373/0x670
    [ 1806.960016] [] ? tcp_push_one+0xa0/0xd0
    [ 1806.960016] [] ? tcp_sendmsg+0x264/0xa30
    [ 1806.960016] [] ? core_sys_select+0x207/0x2c0
    [ 1806.960016] [] ? sock_aio_write+0xeb/0x110
    [ 1806.960016] [] ? do_sync_write+0xcc/0x110
    [ 1806.960016] [] ? pty_unthrottle+0x15/0x20
    [ 1806.960016] [] ? autoremove_wake_function+0x0/0x50
    [ 1806.960016] [] ? current_fs_time+0x16/0x20
    [ 1806.960016] [] ? vfs_write+0x110/0x120
    [ 1806.960016] [] ? sys_write+0x3d/0x70
    [ 1806.960016] [] ? sysenter_do_call+0x12/0x25
    [ 1806.960016] Code: 0f b6 4d 89 89 45 dc 88 4d e0 8b 52 50 29 c2 b8 d0 04 00 00 81 fa d0 04 00 00 0f 47 d0 85 d2 0f 88 91 01 00 00 8b 4d 84 8b 41 14 <8b> 98 24 01 00 00 85 db 74 06 ff 83 80 00 00 00 b8 40 00 00 00
    [ 1806.960016] EIP: [] icmpv6_send+0x387/0x580 SS:ESP 0068:f6babbf8
    [ 1807.067511] Kernel panic - not syncing: Fatal exception in interrupt


    Second attempt, delayed after calling fbset from console:

    [ 217.260426] BUG: unable to handle kernel NULL pointer dereference at 000000c7
    [ 217.260915] IP: [] rt_worker_func+0xb6/0x160
    [ 217.261264] *pde = 00000000
    [ 217.261458] Oops: 0000 [#1]
    [ 217.261649] last sysfs file: /sys/devices/platform/w83627hf.656/temp2_input
    [ 217.262058] Modules linked in: snd_hda_intel snd_pcm snd_timer snd soundcore snd_page_alloc sg
    [ 217.262691]
    [ 217.262795] Pid: 5, comm: events/0 Not tainted (2.6.28-rc3-git6 #1) CX700+W697HG
    [ 217.263236] EIP: 0060:[] EFLAGS: 00010286 CPU: 0
    [ 217.263570] EIP is at rt_worker_func+0xb6/0x160
    [ 217.263846] EAX: 00000002 EBX: ffffffff ECX: c0606e20 EDX: fffffed4
    [ 217.270015] ESI: f7172c5c EDI: 00007530 EBP: f7032f80 ESP: f7032f6c
    [ 217.270015] DS: 007b ES: 007b FS: 0000 GS: 0000 SS: 0068
    [ 217.270015] Process events/0 (pid: 5, ti=f7032000 task=f702ad80 task.ti=f7032000)
    [ 217.270015] Stack:
    [ 217.270015] 000001b5 00004b17 c053d7a0 f7008180 c0380a90 f7032fa4 c0130117 f702b440
    [ 217.270015] f702ad80 c0510180 00000246 f7008188 f7008180 f7032fac f7032fcc c0130747
    [ 217.270015] 00000000 f702ad80 c0133400 f7032fb8 f7032fb8 fffffffc f7008180 c01306b0
    [ 217.270015] Call Trace:
    [ 217.270015] [] ? rt_worker_func+0x0/0x160
    [ 217.270015] [] ? run_workqueue+0x67/0xe0
    [ 217.270015] [] ? worker_thread+0x97/0xf0
    [ 217.270015] [] ? autoremove_wake_function+0x0/0x50
    [ 217.270015] [] ? worker_thread+0x0/0xf0
    [ 217.270015] [] ? kthread+0x42/0x70
    [ 217.270015] [] ? kthread+0x0/0x70
    [ 217.270015] [] ? kernel_thread_helper+0x7/0x10
    [ 217.270015] Code: f0 ff ff f6 40 08 08 0f 85 bb 00 00 00 8b 06 85 c0 74 49 89 df e8 8b 5c da ff 8d 74 26 00 8d bc 27 00 00 00 00 8b 1e 85 db 74 2c <8b> 83 c8 00 00 00 3b 05 dc c9 61 c0 75 4c 8b 53 18 85 d2 74 2c
    [ 217.270015] EIP: [] rt_worker_func+0xb6/0x160 SS:ESP 0068:f7032f6c
    [ 217.526097] Kernel panic - not syncing: Fatal exception in interrupt
    --
    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/

  5. Re: [PATCH] Fix crash in viafb due to 4k stack overflow

    On Sun, 9 Nov 2008 21:38:11 +0100 Bruno Pr__mont wrote:

    > On Sun, 09 November 2008 Arjan van de Ven wrote:
    > > On Sun, 9 Nov 2008 Andrew Morton wrote:
    > > > On Sun, 9 Nov 2008 Bruno Pr__mont wrote:
    > > >
    > > > > The function viafb_cursor() uses 2 stack-variables of CURSOR_SIZE
    > > > > bits; CURSOR_SIZE is defined as (8 * 1024). Using up twice 1k on
    > > > > stack is too much for 4k-stack (though it works with 8k-stacks).
    > > >
    > > > >
    > > > > if (viacursor.enable)
    > > >
    > > > Is the ->fb_cursor handler allowed to perform GFP_KERNEL memory
    > > > allocations? It's never called from atomic contexts?

    > >
    > > if it's allowed to do GFP_KERNEL memory allocations the statement that
    > > it works with 8k stacks is a bit overstated... since irq's can come in
    > > and take several KB of stack as well

    > I don't know if it can be called from atomic contexts or not
    >
    >
    > In addition I get panics some time after start-up which I'm not sure
    > what to associate them with (apparently unrelated)
    > It could be some stack overflow by calling fbset (I will to more testing
    > in order to find out)
    >
    > First attempt: calling fbset via ssh:
    >
    > [ 1806.952151] BUG: unable to handle kernel NULL pointer dereference at 00000123
    > [ 1806.952601] IP: [] icmpv6_send+0x387/0x580
    >
    > ...
    >
    > Second attempt, delayed after calling fbset from console:
    >
    > [ 217.260426] BUG: unable to handle kernel NULL pointer dereference at 000000c7
    > [ 217.260915] IP: [] rt_worker_func+0xb6/0x160


    gack. Your kernel was destroyed.

    Stack overflow might well explain this. Does it work OK with 8k stacks?

    scripts/checkstack.pl should help find the problems.
    --
    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/

  6. Re: [PATCH] Fix crash in viafb due to 4k stack overflow

    On Sun, 09 November 2008 Andrew Morton wrote:
    > On Sun, 9 Nov 2008 21:38:11 +0100 Bruno Prémont wrote:
    > > On Sun, 09 November 2008 Arjan van de Ven wrote:
    > > > On Sun, 9 Nov 2008 Andrew Morton wrote:
    > > > > On Sun, 9 Nov 2008 Bruno Pr__mont wrote:
    > > > >
    > > > > > The function viafb_cursor() uses 2 stack-variables of
    > > > > > CURSOR_SIZE bits; CURSOR_SIZE is defined as (8 * 1024). Using
    > > > > > up twice 1k on stack is too much for 4k-stack (though it
    > > > > > works with 8k-stacks).
    > > > >
    > > > > >
    > > > > > if (viacursor.enable)
    > > > >
    > > > > Is the ->fb_cursor handler allowed to perform GFP_KERNEL memory
    > > > > allocations? It's never called from atomic contexts?
    > > >
    > > > if it's allowed to do GFP_KERNEL memory allocations the statement
    > > > that it works with 8k stacks is a bit overstated... since irq's
    > > > can come in and take several KB of stack as well

    > > I don't know if it can be called from atomic contexts or not


    Ok scanned under drivers/video/ for users of fb_cursor() and all those
    (under drivers/video/console/) do GPT_ATOMIC allocations before calling
    fbops->fb_cursor, so my patch chooses the wrong allocation constraint.

    Fixed patch below

    > > In addition I get panics some time after start-up which I'm not sure
    > > what to associate them with (apparently unrelated)
    > > It could be some stack overflow by calling fbset (I will to more
    > > testing in order to find out)
    > >
    > > First attempt: calling fbset via ssh:
    > >
    > > [ 1806.952151] BUG: unable to handle kernel NULL pointer
    > > dereference at 00000123 [ 1806.952601] IP: []
    > > icmpv6_send+0x387/0x580
    > >
    > > ...
    > >
    > > Second attempt, delayed after calling fbset from console:
    > >
    > > [ 217.260426] BUG: unable to handle kernel NULL pointer
    > > dereference at 000000c7 [ 217.260915] IP: []
    > > rt_worker_func+0xb6/0x160

    >
    > gack. Your kernel was destroyed.
    >
    > Stack overflow might well explain this. Does it work OK with 8k
    > stacks?

    My last attempt with 8k stacks is a bit dated (2.6.27-rc) but back then
    I saw the same kind of behavior than now with viafb, crash with 4k-stack
    but running system with 8k-stack. Running system does of course not mean
    that stack cannot overflow

    > scripts/checkstack.pl should help find the problems.


    Thanks for the pointer!

    It show a nice candidate, viafb_ioctl in top-6:
    0xc011612b identity_mapped [vmlinux]: 4096
    0xc017896b do_sys_poll [vmlinux]: 888
    0xc0178bdd do_sys_poll [vmlinux]: 888
    0xc024506b sha256_transform [vmlinux]: 752
    0xc024768b sha256_transform [vmlinux]: 752
    0xc027d933 viafb_ioctl [vmlinux]: 728
    0xc01783c8 do_select [vmlinux]: 708
    0xc0178853 do_select [vmlinux]: 708


    On a new attempt the box survived fbset "1280x1024-60" though
    I did wait some time after boot up before calling it.
    So it's pretty probable that either it gets near the limit of stack
    or this time the neighborhood of the stack was not just as critical :/

    Shall I trim down that function's stack usage as well?
    (many structs allocated from stack)

    What is preferred, allocating a big block of memory and point various
    variables inside that block or do multiple individual allocations?

    e.g.
    u8 data[CURSOR_SIZE/8]
    u32 data_bak[CURSOR_SIZE/32]
    =>
    u8 *data = kzalloc(...)
    u32 *data_bak = kzalloc(...)
    or
    u8 *data = kzalloc(CURSOR_SIZE/8 + CURSOR_SIZE/32, ...)
    u32 *data_bak = (u32*)(data+CURSOR_SIZE/8);

    First option is more readable, second should be more efficient...

    The end result readability would suffer even more in case of viafb_ioctl()
    with the big amount of different structs that could be allocated from heap
    instead of stack...

    Bruno



    ---
    --- linux-2.6.28-rc3.orig/drivers/video/via/viafbdev.c 2008-11-09 19:22:15.000000000 +0100
    +++ linux-2.6.28-rc3/drivers/video/via/viafbdev.c 2008-11-09 21:25:47.000000000 +0100
    @@ -1052,10 +1052,8 @@ static void viafb_imageblit(struct fb_in

    static int viafb_cursor(struct fb_info *info, struct fb_cursor *cursor)
    {
    - u8 data[CURSOR_SIZE / 8];
    - u32 data_bak[CURSOR_SIZE / 32];
    u32 temp, xx, yy, bg_col = 0, fg_col = 0;
    - int size, i, j = 0;
    + int i, j = 0;
    static int hw_cursor;
    struct viafb_par *p_viafb_par;

    @@ -1178,10 +1176,15 @@ static int viafb_cursor(struct fb_info *
    }

    if (cursor->set & FB_CUR_SETSHAPE) {
    - size =
    + u8 *data = kzalloc(CURSOR_SIZE / 8, GFP_ATOMIC);
    + u32 *data_bak = kzalloc(CURSOR_SIZE / 32, GFP_ATOMIC);
    + int size =
    ((viacursor.image.width + 7) >> 3) *
    viacursor.image.height;

    + if (data == NULL || data_bak == NULL)
    + goto out;
    +
    if (MAX_CURS == 32) {
    for (i = 0; i < (CURSOR_SIZE / 32); i++) {
    data_bak[i] = 0x0;
    @@ -1231,6 +1234,9 @@ static int viafb_cursor(struct fb_info *
    memcpy(((struct viafb_par *)(info->par))->fbmem_virt +
    ((struct viafb_par *)(info->par))->cursor_start,
    data_bak, CURSOR_SIZE);
    +out:
    + kfree(data);
    + kfree(data_bak);
    }

    if (viacursor.enable)
    --
    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/

  7. Re: [PATCH] Fix crash in viafb due to 4k stack overflow

    On Sun, 9 Nov 2008, Bruno Prémont wrote:
    > What is preferred, allocating a big block of memory and point various
    > variables inside that block or do multiple individual allocations?
    >
    > e.g.
    > u8 data[CURSOR_SIZE/8]
    > u32 data_bak[CURSOR_SIZE/32]
    > =>
    > u8 *data = kzalloc(...)
    > u32 *data_bak = kzalloc(...)
    > or
    > u8 *data = kzalloc(CURSOR_SIZE/8 + CURSOR_SIZE/32, ...)
    > u32 *data_bak = (u32*)(data+CURSOR_SIZE/8);
    >
    > First option is more readable, second should be more efficient...


    I like this method:

    foo()
    {
    /* anonymous struct, you don't need to think of a name */
    struct {
    u8 data[CURSOR_SIZE/8];
    u32 data_bak[CURSOR_SIZE/32];
    } *cursor;

    cursor = kzalloc(sizeof(*cursor), ...);

    /* for remaining code:
    * s/data/cursor->data/
    * s/data_bak/cursor->data_bak/
    */

    free(cursor);
    }

    A number of advantages over multiple allocations:
    The alloc code and free code only has to run once.

    Likely less memory will be allocated, due to allocation granularity.

    Only one pointer is needed to keep track of the allocations instead of two.
    This frees up a pointer's worth of stack space and/or another register for
    the compiler to use.

    More readable than u32 *data_bak = (u32*)(data+CURSOR_SIZE/8) and so on.

    If you check for kzalloc failure, you only need code to check once. And
    you can avoid the need for rolling back the first allocation if the second
    fails.

    The disadvantage is that you can't free just one of the allocations and big
    allocations are harder to satisfy than small ones. When you get up to the
    megabyte range, combining allocations into bigger ones becomes a bad idea.

  8. Re: [PATCH] Fix crash in viafb due to 4k stack overflow

    On Sun, 9 Nov 2008 22:37:48 +0100 Bruno Pr__mont wrote:

    >
    > Ok scanned under drivers/video/ for users of fb_cursor() and all those
    > (under drivers/video/console/) do GPT_ATOMIC allocations before calling
    > fbops->fb_cursor, so my patch chooses the wrong allocation constraint.
    >
    > Fixed patch below


    Updated, thanks.

    > > > In addition I get panics some time after start-up which I'm not sure
    > > > what to associate them with (apparently unrelated)
    > > > It could be some stack overflow by calling fbset (I will to more
    > > > testing in order to find out)
    > > >
    > > > First attempt: calling fbset via ssh:
    > > >
    > > > [ 1806.952151] BUG: unable to handle kernel NULL pointer
    > > > dereference at 00000123 [ 1806.952601] IP: []
    > > > icmpv6_send+0x387/0x580
    > > >
    > > > ...
    > > >
    > > > Second attempt, delayed after calling fbset from console:
    > > >
    > > > [ 217.260426] BUG: unable to handle kernel NULL pointer
    > > > dereference at 000000c7 [ 217.260915] IP: []
    > > > rt_worker_func+0xb6/0x160

    > >
    > > gack. Your kernel was destroyed.
    > >
    > > Stack overflow might well explain this. Does it work OK with 8k
    > > stacks?

    > My last attempt with 8k stacks is a bit dated (2.6.27-rc) but back then
    > I saw the same kind of behavior than now with viafb, crash with 4k-stack
    > but running system with 8k-stack. Running system does of course not mean
    > that stack cannot overflow
    >
    > > scripts/checkstack.pl should help find the problems.

    >
    > Thanks for the pointer!
    >
    > It show a nice candidate, viafb_ioctl in top-6:
    > 0xc011612b identity_mapped [vmlinux]: 4096


    erk!

    > 0xc017896b do_sys_poll [vmlinux]: 888
    > 0xc0178bdd do_sys_poll [vmlinux]: 888
    > 0xc024506b sha256_transform [vmlinux]: 752
    > 0xc024768b sha256_transform [vmlinux]: 752
    > 0xc027d933 viafb_ioctl [vmlinux]: 728
    > 0xc01783c8 do_select [vmlinux]: 708
    > 0xc0178853 do_select [vmlinux]: 708
    >
    >
    > On a new attempt the box survived fbset "1280x1024-60" though
    > I did wait some time after boot up before calling it.
    > So it's pretty probable that either it gets near the limit of stack
    > or this time the neighborhood of the stack was not just as critical :/
    >
    > Shall I trim down that function's stack usage as well?
    > (many structs allocated from stack)


    Yes please.

    > What is preferred, allocating a big block of memory and point various
    > variables inside that block or do multiple individual allocations?
    >
    > e.g.
    > u8 data[CURSOR_SIZE/8]
    > u32 data_bak[CURSOR_SIZE/32]
    > =>
    > u8 *data = kzalloc(...)
    > u32 *data_bak = kzalloc(...)
    > or
    > u8 *data = kzalloc(CURSOR_SIZE/8 + CURSOR_SIZE/32, ...)
    > u32 *data_bak = (u32*)(data+CURSOR_SIZE/8);
    >
    > First option is more readable, second should be more efficient...


    If the total allocation is greater than 4096 then it will need a
    larger-than-order-zero allocation, and there are some reliability
    risks there. But if it's 4096 we're OK.

    A good way to code this would be

    struct {
    u8 data[CURSOR_SIZE/8];
    u32 data_bak[CURSOR_SIZE/32];
    ...
    } *local_data = kzalloc(sizeof(*local_data, GFP_WHATEVER));

    if (!local_data)
    bummer();
    ...
    local_data->data = foo;
    ...
    kfree(local_data);

    where GFP_WHATEVER should be the reliable GFP_KERNEL if possible, and
    the unreliable GFP_ATOMIC otherwise.

    > The end result readability would suffer even more in case of viafb_ioctl()
    > with the big amount of different structs that could be allocated from heap
    > instead of stack...


    Perhaps the above texhnique could be used there as well.


    --
    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/

  9. Re: [PATCH] Fix crash in viafb due to 4k stack overflow

    On Sun, 09 November 2008 Andrew Morton wrote:
    > > It show a nice candidate, viafb_ioctl in top-6:
    > > 0xc011612b identity_mapped [vmlinux]: 4096

    >
    > erk!
    >
    > > 0xc017896b do_sys_poll [vmlinux]: 888
    > > 0xc0178bdd do_sys_poll [vmlinux]: 888
    > > 0xc024506b sha256_transform [vmlinux]: 752
    > > 0xc024768b sha256_transform [vmlinux]: 752
    > > 0xc027d933 viafb_ioctl [vmlinux]: 728
    > > 0xc01783c8 do_select [vmlinux]: 708
    > > 0xc0178853 do_select [vmlinux]: 708
    > >
    > >
    > > On a new attempt the box survived fbset "1280x1024-60" though
    > > I did wait some time after boot up before calling it.
    > > So it's pretty probable that either it gets near the limit of stack
    > > or this time the neighborhood of the stack was not just as
    > > critical :/
    > >
    > > Shall I trim down that function's stack usage as well?
    > > (many structs allocated from stack)

    >
    > Yes please.


    I did a first attempt, based on suggested technique.
    For viafb_ioctl() I did put all the structs in a single union (on stack)
    and used the appropriate one depending on the cmd.
    This saves 276 bytes out of 728 according to check_stack.pl

    The most clean approach would probably be to split out the parts using
    bigger structs into separate functions and allocating the required
    memory from there.

    I also switched viafb_cursor() to the struct technique.

    See patch below for both.




    During conversion of viafb_ioctl() I noticed the following:

    Those two cases just copy_from_user and do nothing with copied data,
    incomplete implementation?:
    case VIAFB_SET_PANEL_POSITION:
    if (copy_from_user(&u.panel_pos_size_para, argp,
    sizeof(u.panel_pos_size_para)))
    return -EFAULT;
    break;
    case VIAFB_SET_PANEL_SIZE:
    if (copy_from_user(&u.panel_pos_size_para, argp,
    sizeof(u.panel_pos_size_para)))
    return -EFAULT;
    break;

    Handling of GET/SET GAMMA looks buggy:
    In each case 256*4 bytes are allocated but only 4 bytes (size of pointer)
    are copied to/from userspace though viafb_(get|set)_gamma_table() operates
    on the full 256 elements...
    case VIAFB_SET_GAMMA_LUT:
    viafb_gamma_table = kmalloc(256 * sizeof(u32), GFP_KERNEL);
    if (!viafb_gamma_table)
    return -ENOMEM;
    if (copy_from_user(viafb_gamma_table, argp,
    sizeof(viafb_gamma_table))) {
    kfree(viafb_gamma_table);
    return -EFAULT;
    }
    viafb_set_gamma_table(viafb_bpp, viafb_gamma_table);
    kfree(viafb_gamma_table);
    break;

    case VIAFB_GET_GAMMA_LUT:
    viafb_gamma_table = kmalloc(256 * sizeof(u32), GFP_KERNEL);
    if (!viafb_gamma_table)
    return -ENOMEM;
    viafb_get_gamma_table(viafb_gamma_table);
    if (copy_to_user(argp, viafb_gamma_table,
    sizeof(viafb_gamma_table))) {
    kfree(viafb_gamma_table);
    return -EFAULT;
    }
    kfree(viafb_gamma_table);
    break;

    I don't know if there is a userspace app that uses these VIA IOCTLs...
    so the ioctl part is just compile-tested.
    After checking, fbset just uses some generic framebuffer IOCTLs outside
    of viafb's scope, thus not passing through viafb_ioctl().



    ---
    --- linux-2.6.28-rc3/drivers/video/via/viafbdev.c.orig 2008-11-09 19:36:47.000000000 +0100
    +++ linux-2.6.28-rc3/drivers/video/via/viafbdev.c 2008-11-10 20:50:32.000000000 +0100
    @@ -546,23 +546,25 @@ static int viafb_blank(int blank_mode, s

    static int viafb_ioctl(struct fb_info *info, u_int cmd, u_long arg)
    {
    - struct viafb_ioctl_mode viamode;
    - struct viafb_ioctl_samm viasamm;
    - struct viafb_driver_version driver_version;
    - struct fb_var_screeninfo sec_var;
    - struct _panel_size_pos_info panel_pos_size_para;
    + union {
    + struct viafb_ioctl_mode viamode;
    + struct viafb_ioctl_samm viasamm;
    + struct viafb_driver_version driver_version;
    + struct fb_var_screeninfo sec_var;
    + struct _panel_size_pos_info panel_pos_size_para;
    + struct viafb_ioctl_setting viafb_setting;
    + struct device_t active_dev;
    + } u;
    u32 state_info = 0;
    - u32 viainfo_size = sizeof(struct viafb_ioctl_info);
    u32 *viafb_gamma_table;
    char driver_name[] = "viafb";

    u32 __user *argp = (u32 __user *) arg;
    u32 gpu32;
    u32 video_dev_info = 0;
    - struct viafb_ioctl_setting viafb_setting = {};
    - struct device_t active_dev = {};

    DEBUG_MSG(KERN_INFO "viafb_ioctl: 0x%X !!\n", cmd);
    + memset(&u, 0, sizeof(u));

    switch (cmd) {
    case VIAFB_GET_CHIP_INFO:
    @@ -571,7 +573,7 @@ static int viafb_ioctl(struct fb_info *i
    return -EFAULT;
    break;
    case VIAFB_GET_INFO_SIZE:
    - return put_user(viainfo_size, argp);
    + return put_user((u32)sizeof(struct viafb_ioctl_info), argp);
    case VIAFB_GET_INFO:
    return viafb_ioctl_get_viafb_info(arg);
    case VIAFB_HOTPLUG:
    @@ -584,60 +586,60 @@ static int viafb_ioctl(struct fb_info *i
    viafb_hotplug = (gpu32) ? 1 : 0;
    break;
    case VIAFB_GET_RESOLUTION:
    - viamode.xres = (u32) viafb_hotplug_Xres;
    - viamode.yres = (u32) viafb_hotplug_Yres;
    - viamode.refresh = (u32) viafb_hotplug_refresh;
    - viamode.bpp = (u32) viafb_hotplug_bpp;
    + u.viamode.xres = (u32) viafb_hotplug_Xres;
    + u.viamode.yres = (u32) viafb_hotplug_Yres;
    + u.viamode.refresh = (u32) viafb_hotplug_refresh;
    + u.viamode.bpp = (u32) viafb_hotplug_bpp;
    if (viafb_SAMM_ON == 1) {
    - viamode.xres_sec = viafb_second_xres;
    - viamode.yres_sec = viafb_second_yres;
    - viamode.virtual_xres_sec = viafb_second_virtual_xres;
    - viamode.virtual_yres_sec = viafb_second_virtual_yres;
    - viamode.refresh_sec = viafb_refresh1;
    - viamode.bpp_sec = viafb_bpp1;
    + u.viamode.xres_sec = viafb_second_xres;
    + u.viamode.yres_sec = viafb_second_yres;
    + u.viamode.virtual_xres_sec = viafb_second_virtual_xres;
    + u.viamode.virtual_yres_sec = viafb_second_virtual_yres;
    + u.viamode.refresh_sec = viafb_refresh1;
    + u.viamode.bpp_sec = viafb_bpp1;
    } else {
    - viamode.xres_sec = 0;
    - viamode.yres_sec = 0;
    - viamode.virtual_xres_sec = 0;
    - viamode.virtual_yres_sec = 0;
    - viamode.refresh_sec = 0;
    - viamode.bpp_sec = 0;
    + u.viamode.xres_sec = 0;
    + u.viamode.yres_sec = 0;
    + u.viamode.virtual_xres_sec = 0;
    + u.viamode.virtual_yres_sec = 0;
    + u.viamode.refresh_sec = 0;
    + u.viamode.bpp_sec = 0;
    }
    - if (copy_to_user(argp, &viamode, sizeof(viamode)))
    + if (copy_to_user(argp, &u.viamode, sizeof(u.viamode)))
    return -EFAULT;
    break;
    case VIAFB_GET_SAMM_INFO:
    - viasamm.samm_status = viafb_SAMM_ON;
    + u.viasamm.samm_status = viafb_SAMM_ON;

    if (viafb_SAMM_ON == 1) {
    if (viafb_dual_fb) {
    - viasamm.size_prim = viaparinfo->fbmem_free;
    - viasamm.size_sec = viaparinfo1->fbmem_free;
    + u.viasamm.size_prim = viaparinfo->fbmem_free;
    + u.viasamm.size_sec = viaparinfo1->fbmem_free;
    } else {
    if (viafb_second_size) {
    - viasamm.size_prim =
    + u.viasamm.size_prim =
    viaparinfo->fbmem_free -
    viafb_second_size * 1024 * 1024;
    - viasamm.size_sec =
    + u.viasamm.size_sec =
    viafb_second_size * 1024 * 1024;
    } else {
    - viasamm.size_prim =
    + u.viasamm.size_prim =
    viaparinfo->fbmem_free >> 1;
    - viasamm.size_sec =
    + u.viasamm.size_sec =
    (viaparinfo->fbmem_free >> 1);
    }
    }
    - viasamm.mem_base = viaparinfo->fbmem;
    - viasamm.offset_sec = viafb_second_offset;
    + u.viasamm.mem_base = viaparinfo->fbmem;
    + u.viasamm.offset_sec = viafb_second_offset;
    } else {
    - viasamm.size_prim =
    + u.viasamm.size_prim =
    viaparinfo->memsize - viaparinfo->fbmem_used;
    - viasamm.size_sec = 0;
    - viasamm.mem_base = viaparinfo->fbmem;
    - viasamm.offset_sec = 0;
    + u.viasamm.size_sec = 0;
    + u.viasamm.mem_base = viaparinfo->fbmem;
    + u.viasamm.offset_sec = 0;
    }

    - if (copy_to_user(argp, &viasamm, sizeof(viasamm)))
    + if (copy_to_user(argp, &u.viasamm, sizeof(u.viasamm)))
    return -EFAULT;

    break;
    @@ -662,74 +664,75 @@ static int viafb_ioctl(struct fb_info *i
    viafb_lcd_disable();
    break;
    case VIAFB_SET_DEVICE:
    - if (copy_from_user(&active_dev, (void *)argp,
    - sizeof(active_dev)))
    + if (copy_from_user(&u.active_dev, (void *)argp,
    + sizeof(u.active_dev)))
    return -EFAULT;
    - viafb_set_device(active_dev);
    + viafb_set_device(u.active_dev);
    viafb_set_par(info);
    break;
    case VIAFB_GET_DEVICE:
    - active_dev.crt = viafb_CRT_ON;
    - active_dev.dvi = viafb_DVI_ON;
    - active_dev.lcd = viafb_LCD_ON;
    - active_dev.samm = viafb_SAMM_ON;
    - active_dev.primary_dev = viafb_primary_dev;
    -
    - active_dev.lcd_dsp_cent = viafb_lcd_dsp_method;
    - active_dev.lcd_panel_id = viafb_lcd_panel_id;
    - active_dev.lcd_mode = viafb_lcd_mode;
    -
    - active_dev.xres = viafb_hotplug_Xres;
    - active_dev.yres = viafb_hotplug_Yres;
    -
    - active_dev.xres1 = viafb_second_xres;
    - active_dev.yres1 = viafb_second_yres;
    -
    - active_dev.bpp = viafb_bpp;
    - active_dev.bpp1 = viafb_bpp1;
    - active_dev.refresh = viafb_refresh;
    - active_dev.refresh1 = viafb_refresh1;
    -
    - active_dev.epia_dvi = viafb_platform_epia_dvi;
    - active_dev.lcd_dual_edge = viafb_device_lcd_dualedge;
    - active_dev.bus_width = viafb_bus_width;
    + u.active_dev.crt = viafb_CRT_ON;
    + u.active_dev.dvi = viafb_DVI_ON;
    + u.active_dev.lcd = viafb_LCD_ON;
    + u.active_dev.samm = viafb_SAMM_ON;
    + u.active_dev.primary_dev = viafb_primary_dev;
    +
    + u.active_dev.lcd_dsp_cent = viafb_lcd_dsp_method;
    + u.active_dev.lcd_panel_id = viafb_lcd_panel_id;
    + u.active_dev.lcd_mode = viafb_lcd_mode;
    +
    + u.active_dev.xres = viafb_hotplug_Xres;
    + u.active_dev.yres = viafb_hotplug_Yres;
    +
    + u.active_dev.xres1 = viafb_second_xres;
    + u.active_dev.yres1 = viafb_second_yres;
    +
    + u.active_dev.bpp = viafb_bpp;
    + u.active_dev.bpp1 = viafb_bpp1;
    + u.active_dev.refresh = viafb_refresh;
    + u.active_dev.refresh1 = viafb_refresh1;
    +
    + u.active_dev.epia_dvi = viafb_platform_epia_dvi;
    + u.active_dev.lcd_dual_edge = viafb_device_lcd_dualedge;
    + u.active_dev.bus_width = viafb_bus_width;

    - if (copy_to_user(argp, &active_dev, sizeof(active_dev)))
    + if (copy_to_user(argp, &u.active_dev, sizeof(u.active_dev)))
    return -EFAULT;
    break;

    case VIAFB_GET_DRIVER_VERSION:
    - driver_version.iMajorNum = VERSION_MAJOR;
    - driver_version.iKernelNum = VERSION_KERNEL;
    - driver_version.iOSNum = VERSION_OS;
    - driver_version.iMinorNum = VERSION_MINOR;
    + u.driver_version.iMajorNum = VERSION_MAJOR;
    + u.driver_version.iKernelNum = VERSION_KERNEL;
    + u.driver_version.iOSNum = VERSION_OS;
    + u.driver_version.iMinorNum = VERSION_MINOR;

    - if (copy_to_user(argp, &driver_version,
    - sizeof(driver_version)))
    + if (copy_to_user(argp, &u.driver_version,
    + sizeof(u.driver_version)))
    return -EFAULT;

    break;

    case VIAFB_SET_DEVICE_INFO:
    - if (copy_from_user(&viafb_setting,
    - argp, sizeof(viafb_setting)))
    + if (copy_from_user(&u.viafb_setting,
    + argp, sizeof(u.viafb_setting)))
    return -EFAULT;
    - if (apply_device_setting(viafb_setting, info) < 0)
    + if (apply_device_setting(u.viafb_setting, info) < 0)
    return -EINVAL;

    break;

    case VIAFB_SET_SECOND_MODE:
    - if (copy_from_user(&sec_var, argp, sizeof(sec_var)))
    + if (copy_from_user(&u.sec_var, argp, sizeof(u.sec_var)))
    return -EFAULT;
    - apply_second_mode_setting(&sec_var);
    + apply_second_mode_setting(&u.sec_var);
    break;

    case VIAFB_GET_DEVICE_INFO:

    - retrieve_device_setting(&viafb_setting);
    + retrieve_device_setting(&u.viafb_setting);

    - if (copy_to_user(argp, &viafb_setting, sizeof(viafb_setting)))
    + if (copy_to_user(argp, &u.viafb_setting,
    + sizeof(u.viafb_setting)))
    return -EFAULT;

    break;
    @@ -806,51 +809,51 @@ static int viafb_ioctl(struct fb_info *i
    break;

    case VIAFB_GET_PANEL_MAX_SIZE:
    - if (copy_from_user
    - (&panel_pos_size_para, argp, sizeof(panel_pos_size_para)))
    + if (copy_from_user(&u.panel_pos_size_para, argp,
    + sizeof(u.panel_pos_size_para)))
    return -EFAULT;
    - panel_pos_size_para.x = panel_pos_size_para.y = 0;
    - if (copy_to_user(argp, &panel_pos_size_para,
    - sizeof(panel_pos_size_para)))
    + u.panel_pos_size_para.x = u.panel_pos_size_para.y = 0;
    + if (copy_to_user(argp, &u.panel_pos_size_para,
    + sizeof(u.panel_pos_size_para)))
    return -EFAULT;
    break;
    case VIAFB_GET_PANEL_MAX_POSITION:
    - if (copy_from_user
    - (&panel_pos_size_para, argp, sizeof(panel_pos_size_para)))
    + if (copy_from_user(&u.panel_pos_size_para, argp,
    + sizeof(u.panel_pos_size_para)))
    return -EFAULT;
    - panel_pos_size_para.x = panel_pos_size_para.y = 0;
    - if (copy_to_user(argp, &panel_pos_size_para,
    - sizeof(panel_pos_size_para)))
    + u.panel_pos_size_para.x = u.panel_pos_size_para.y = 0;
    + if (copy_to_user(argp, &u.panel_pos_size_para,
    + sizeof(u.panel_pos_size_para)))
    return -EFAULT;
    break;

    case VIAFB_GET_PANEL_POSITION:
    - if (copy_from_user
    - (&panel_pos_size_para, argp, sizeof(panel_pos_size_para)))
    + if (copy_from_user(&u.panel_pos_size_para, argp,
    + sizeof(u.panel_pos_size_para)))
    return -EFAULT;
    - panel_pos_size_para.x = panel_pos_size_para.y = 0;
    - if (copy_to_user(argp, &panel_pos_size_para,
    - sizeof(panel_pos_size_para)))
    + u.panel_pos_size_para.x = u.panel_pos_size_para.y = 0;
    + if (copy_to_user(argp, &u.panel_pos_size_para,
    + sizeof(u.panel_pos_size_para)))
    return -EFAULT;
    break;
    case VIAFB_GET_PANEL_SIZE:
    - if (copy_from_user
    - (&panel_pos_size_para, argp, sizeof(panel_pos_size_para)))
    + if (copy_from_user(&u.panel_pos_size_para, argp,
    + sizeof(u.panel_pos_size_para)))
    return -EFAULT;
    - panel_pos_size_para.x = panel_pos_size_para.y = 0;
    - if (copy_to_user(argp, &panel_pos_size_para,
    - sizeof(panel_pos_size_para)))
    + u.panel_pos_size_para.x = u.panel_pos_size_para.y = 0;
    + if (copy_to_user(argp, &u.panel_pos_size_para,
    + sizeof(u.panel_pos_size_para)))
    return -EFAULT;
    break;

    case VIAFB_SET_PANEL_POSITION:
    - if (copy_from_user
    - (&panel_pos_size_para, argp, sizeof(panel_pos_size_para)))
    + if (copy_from_user(&u.panel_pos_size_para, argp,
    + sizeof(u.panel_pos_size_para)))
    return -EFAULT;
    break;
    case VIAFB_SET_PANEL_SIZE:
    - if (copy_from_user
    - (&panel_pos_size_para, argp, sizeof(panel_pos_size_para)))
    + if (copy_from_user(&u.panel_pos_size_para, argp,
    + sizeof(u.panel_pos_size_para)))
    return -EFAULT;
    break;

    @@ -1052,10 +1055,8 @@ static void viafb_imageblit(struct fb_in

    static int viafb_cursor(struct fb_info *info, struct fb_cursor *cursor)
    {
    - u8 data[CURSOR_SIZE / 8];
    - u32 data_bak[CURSOR_SIZE / 32];
    u32 temp, xx, yy, bg_col = 0, fg_col = 0;
    - int size, i, j = 0;
    + int i, j = 0;
    static int hw_cursor;
    struct viafb_par *p_viafb_par;

    @@ -1178,22 +1179,29 @@ static int viafb_cursor(struct fb_info *
    }

    if (cursor->set & FB_CUR_SETSHAPE) {
    - size =
    + struct {
    + u8 data[CURSOR_SIZE / 8];
    + u32 bak[CURSOR_SIZE / 32];
    + } *cr_data = kzalloc(sizeof(*cr_data), GFP_ATOMIC);
    + int size =
    ((viacursor.image.width + 7) >> 3) *
    viacursor.image.height;

    + if (cr_data == NULL)
    + goto out;
    +
    if (MAX_CURS == 32) {
    for (i = 0; i < (CURSOR_SIZE / 32); i++) {
    - data_bak[i] = 0x0;
    - data_bak[i + 1] = 0xFFFFFFFF;
    + cr_data->bak[i] = 0x0;
    + cr_data->bak[i + 1] = 0xFFFFFFFF;
    i += 1;
    }
    } else if (MAX_CURS == 64) {
    for (i = 0; i < (CURSOR_SIZE / 32); i++) {
    - data_bak[i] = 0x0;
    - data_bak[i + 1] = 0x0;
    - data_bak[i + 2] = 0xFFFFFFFF;
    - data_bak[i + 3] = 0xFFFFFFFF;
    + cr_data->bak[i] = 0x0;
    + cr_data->bak[i + 1] = 0x0;
    + cr_data->bak[i + 2] = 0xFFFFFFFF;
    + cr_data->bak[i + 3] = 0xFFFFFFFF;
    i += 3;
    }
    }
    @@ -1201,12 +1209,12 @@ static int viafb_cursor(struct fb_info *
    switch (viacursor.rop) {
    case ROP_XOR:
    for (i = 0; i < size; i++)
    - data[i] = viacursor.mask[i];
    + cr_data->data[i] = viacursor.mask[i];
    break;
    case ROP_COPY:

    for (i = 0; i < size; i++)
    - data[i] = viacursor.mask[i];
    + cr_data->data[i] = viacursor.mask[i];
    break;
    default:
    break;
    @@ -1214,23 +1222,25 @@ static int viafb_cursor(struct fb_info *

    if (MAX_CURS == 32) {
    for (i = 0; i < size; i++) {
    - data_bak[j] = (u32) data[i];
    - data_bak[j + 1] = ~data_bak[j];
    + cr_data->bak[j] = (u32) cr_data->data[i];
    + cr_data->bak[j + 1] = ~cr_data->bak[j];
    j += 2;
    }
    } else if (MAX_CURS == 64) {
    for (i = 0; i < size; i++) {
    - data_bak[j] = (u32) data[i];
    - data_bak[j + 1] = 0x0;
    - data_bak[j + 2] = ~data_bak[j];
    - data_bak[j + 3] = ~data_bak[j + 1];
    + cr_data->bak[j] = (u32) cr_data->data[i];
    + cr_data->bak[j + 1] = 0x0;
    + cr_data->bak[j + 2] = ~cr_data->bak[j];
    + cr_data->bak[j + 3] = ~cr_data->bak[j + 1];
    j += 4;
    }
    }

    memcpy(((struct viafb_par *)(info->par))->fbmem_virt +
    ((struct viafb_par *)(info->par))->cursor_start,
    - data_bak, CURSOR_SIZE);
    + cr_data->bak, CURSOR_SIZE);
    +out:
    + kfree(cr_data);
    }

    if (viacursor.enable)
    --
    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/

+ Reply to Thread