Skip to content

Commit

Permalink
dlm: fix length calculation in compat code
Browse files Browse the repository at this point in the history
Using offsetof() to calculate name length does not work because
it does not produce consistent results with with structure packing.
This caused memcpy to corrupt memory by copying 4 extra bytes off
the end of the buffer on 64 bit kernels with 32 bit userspace
(the only case where this 32/64 compat code is used).

The fix is to calculate name length directly from the start instead
of trying to derive it later using count and offsetof.

Signed-off-by: David Teigland <teigland@redhat.com>
  • Loading branch information
David Teigland committed Mar 11, 2009
1 parent a536e38 commit 1fecb1c
Showing 1 changed file with 15 additions and 9 deletions.
24 changes: 15 additions & 9 deletions fs/dlm/user.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2006-2008 Red Hat, Inc. All rights reserved.
* Copyright (C) 2006-2009 Red Hat, Inc. All rights reserved.
*
* This copyrighted material is made available to anyone wishing to use,
* modify, copy, or redistribute it subject to the terms and conditions
Expand Down Expand Up @@ -84,7 +84,7 @@ struct dlm_lock_result32 {

static void compat_input(struct dlm_write_request *kb,
struct dlm_write_request32 *kb32,
size_t count)
int namelen)
{
kb->version[0] = kb32->version[0];
kb->version[1] = kb32->version[1];
Expand All @@ -96,8 +96,7 @@ static void compat_input(struct dlm_write_request *kb,
kb->cmd == DLM_USER_REMOVE_LOCKSPACE) {
kb->i.lspace.flags = kb32->i.lspace.flags;
kb->i.lspace.minor = kb32->i.lspace.minor;
memcpy(kb->i.lspace.name, kb32->i.lspace.name, count -
offsetof(struct dlm_write_request32, i.lspace.name));
memcpy(kb->i.lspace.name, kb32->i.lspace.name, namelen);
} else if (kb->cmd == DLM_USER_PURGE) {
kb->i.purge.nodeid = kb32->i.purge.nodeid;
kb->i.purge.pid = kb32->i.purge.pid;
Expand All @@ -115,8 +114,7 @@ static void compat_input(struct dlm_write_request *kb,
kb->i.lock.bastaddr = (void *)(long)kb32->i.lock.bastaddr;
kb->i.lock.lksb = (void *)(long)kb32->i.lock.lksb;
memcpy(kb->i.lock.lvb, kb32->i.lock.lvb, DLM_USER_LVB_LEN);
memcpy(kb->i.lock.name, kb32->i.lock.name, count -
offsetof(struct dlm_write_request32, i.lock.name));
memcpy(kb->i.lock.name, kb32->i.lock.name, namelen);
}
}

Expand Down Expand Up @@ -539,17 +537,25 @@ static ssize_t device_write(struct file *file, const char __user *buf,
#ifdef CONFIG_COMPAT
if (!kbuf->is64bit) {
struct dlm_write_request32 *k32buf;
int namelen = 0;

if (count > sizeof(struct dlm_write_request32))
namelen = count - sizeof(struct dlm_write_request32);

k32buf = (struct dlm_write_request32 *)kbuf;
kbuf = kmalloc(count + 1 + (sizeof(struct dlm_write_request) -
sizeof(struct dlm_write_request32)), GFP_KERNEL);

/* add 1 after namelen so that the name string is terminated */
kbuf = kzalloc(sizeof(struct dlm_write_request) + namelen + 1,
GFP_KERNEL);
if (!kbuf) {
kfree(k32buf);
return -ENOMEM;
}

if (proc)
set_bit(DLM_PROC_FLAGS_COMPAT, &proc->flags);
compat_input(kbuf, k32buf, count + 1);

compat_input(kbuf, k32buf, namelen);
kfree(k32buf);
}
#endif
Expand Down

0 comments on commit 1fecb1c

Please sign in to comment.