lkml.org 
[lkml]   [2009]   [Feb]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patches in this message
    /
    Date
    From
    Subject[PATCH] sgi-xp: fix writing past the end of kzalloc()'d space (v.2)
    A missing type cast results in writing way beyond the end of a kzalloc()'d
    memory segment resulting in slab corruption. But it seems like the better
    solution is to define ->recv_msg_slots as a 'void *' rather than a
    'struct xpc_notify_mq_msg_uv *' and add the type cast.

    Signed-off-by: Dean Nelson <dcn@sgi.com>
    Cc: stable <stable@kernel.org>

    ---

    On Tue, Feb 03, 2009 at 12:01:47PM -0800, Andrew Morton wrote:
    > On Tue, 3 Feb 2009 12:23:38 -0600
    > Dean Nelson <dcn@sgi.com> wrote:
    >
    > > --- linux.orig/drivers/misc/sgi-xp/xpc_uv.c 2009-01-27 10:53:26.000000000 -0600
    > > +++ linux/drivers/misc/sgi-xp/xpc_uv.c 2009-01-28 08:51:57.000000000 -0600
    :
    > > @@ -1129,8 +1129,8 @@ xpc_allocate_recv_msg_slot_uv(struct xpc
    > > continue;
    > >
    > > for (entry = 0; entry < nentries; entry++) {
    > > - msg_slot = ch_uv->recv_msg_slots + entry *
    > > - ch->entry_size;
    > > + msg_slot = (struct xpc_notify_mq_msg_uv *)((u8 *)
    > > + ch_uv->recv_msg_slots + entry * ch->entry_size);
    >
    > So.. ch->entry_size is not equal to sizeof(struct xpc_notify_mq_msg_uv)?

    Correct. The value of ch->entry_size is defined at runtime by the user of
    XPC (i.e., XPNET and XPMEM) when it calls xpc_connect(). It is always larger
    than sizeof(struct xpc_notify_mq_msg_uv) which is defined as:

    struct xpc_notify_mq_msg_uv {
    struct xpc_notify_mq_msghdr_uv hdr;
    unsigned long payload;
    };

    The payload field reflects the first eight bytes of the user's payload
    which extends into the remainder of the messsge slot.

    > Perhaps ->recv_msg_slots should never have had type struct
    > xpc_notify_mq_msg_uv *?

    Agreed. So I'm resubmitting the patch based on this recommendation.

    Note that I'm using 'void *' instead of 'u8 *' to ->define recv_msg_slots.
    Currently they are equivalent in terms of the math, but 'void *' has the
    advantage of not needing to cast at all. Is this an appropriate choice?
    Thanks,
    Dean

    drivers/misc/sgi-xp/xpc.h | 5 +++--
    drivers/misc/sgi-xp/xpc_uv.c | 11 +++++------
    2 files changed, 8 insertions(+), 8 deletions(-)
    Index: linux/drivers/misc/sgi-xp/xpc.h
    ===================================================================
    --- linux.orig/drivers/misc/sgi-xp/xpc.h 2009-02-04 09:30:24.000000000 -0600
    +++ linux/drivers/misc/sgi-xp/xpc.h 2009-02-04 11:33:48.000000000 -0600
    @@ -3,7 +3,7 @@
    * License. See the file "COPYING" in the main directory of this archive
    * for more details.
    *
    - * Copyright (c) 2004-2008 Silicon Graphics, Inc. All Rights Reserved.
    + * Copyright (c) 2004-2009 Silicon Graphics, Inc. All Rights Reserved.
    */

    /*
    @@ -519,7 +519,8 @@ struct xpc_channel_uv {
    /* gru mq descriptor */

    struct xpc_send_msg_slot_uv *send_msg_slots;
    - struct xpc_notify_mq_msg_uv *recv_msg_slots;
    + void *recv_msg_slots; /* each slot will hold a xpc_notify_mq_msg_uv */
    + /* structure plus the user's payload */

    struct xpc_fifo_head_uv msg_slot_free_list;
    struct xpc_fifo_head_uv recv_msg_list; /* deliverable payloads */
    Index: linux/drivers/misc/sgi-xp/xpc_uv.c
    ===================================================================
    --- linux.orig/drivers/misc/sgi-xp/xpc_uv.c 2009-01-27 10:53:26.000000000 -0600
    +++ linux/drivers/misc/sgi-xp/xpc_uv.c 2009-02-04 11:33:33.000000000 -0600
    @@ -3,7 +3,7 @@
    * License. See the file "COPYING" in the main directory of this archive
    * for more details.
    *
    - * Copyright (c) 2008 Silicon Graphics, Inc. All Rights Reserved.
    + * Copyright (c) 2008-2009 Silicon Graphics, Inc. All Rights Reserved.
    */

    /*
    @@ -1129,8 +1129,8 @@ xpc_allocate_recv_msg_slot_uv(struct xpc
    continue;

    for (entry = 0; entry < nentries; entry++) {
    - msg_slot = ch_uv->recv_msg_slots + entry *
    - ch->entry_size;
    + msg_slot = ch_uv->recv_msg_slots +
    + entry * ch->entry_size;

    msg_slot->hdr.msg_slot_number = entry;
    }
    @@ -1438,9 +1438,8 @@ xpc_handle_notify_mq_msg_uv(struct xpc_p
    /* we're dealing with a normal message sent via the notify_mq */
    ch_uv = &ch->sn.uv;

    - msg_slot = (struct xpc_notify_mq_msg_uv *)((u64)ch_uv->recv_msg_slots +
    - (msg->hdr.msg_slot_number % ch->remote_nentries) *
    - ch->entry_size);
    + msg_slot = ch_uv->recv_msg_slots +
    + (msg->hdr.msg_slot_number % ch->remote_nentries) * ch->entry_size;

    BUG_ON(msg->hdr.msg_slot_number != msg_slot->hdr.msg_slot_number);
    BUG_ON(msg_slot->hdr.size != 0);

    \
     
     \ /
      Last update: 2009-02-04 19:39    [W:0.025 / U:0.480 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site