Skip to content

Commit

Permalink
connector: improved unaligned access error fix
Browse files Browse the repository at this point in the history 
In af3e095, Erik Jacobsen fixed one type of unaligned access
bug for ia64 by converting a 64-bit write to use put_unaligned().
Unfortunately, since gcc will convert a short memset() to a series
of appropriately-aligned stores, the problem is now visible again
on tilegx, where the memset that zeros out proc_event is converted
to three 64-bit stores, causing an unaligned access panic.

A better fix for the original problem is to ensure that proc_event
is aligned to 8 bytes here.  We can do that relatively easily by
arranging to start the struct cn_msg aligned to 8 bytes and then
offset by 4 bytes.  Doing so means that the immediately following
proc_event structure is then correctly aligned to 8 bytes.

The result is that the memset() stores are now aligned, and as an
added benefit, we can remove the put_unaligned() calls in the code.

Signed-off-by: Chris Metcalf <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
  • Loading branch information
@cmetcalf-tilera @davem330
cmetcalf-tilera authored and davem330 committed Nov 14, 2013
1 parent 2abc2f0 commit 1ca1a4c
Showing 1 changed file with 42 additions and 30 deletions.
72 changes: 42 additions & 30 deletions drivers/connector/cn_proc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,23 @@
#include <linux/atomic.h>
#include <linux/pid_namespace.h>

#include <asm/unaligned.h>

#include <linux/cn_proc.h>

#define CN_PROC_MSG_SIZE (sizeof(struct cn_msg) + sizeof(struct proc_event))
/*
* Size of a cn_msg followed by a proc_event structure. Since the
* sizeof struct cn_msg is a multiple of 4 bytes, but not 8 bytes, we
* add one 4-byte word to the size here, and then start the actual
* cn_msg structure 4 bytes into the stack buffer. The result is that
* the immediately following proc_event structure is aligned to 8 bytes.
*/
#define CN_PROC_MSG_SIZE (sizeof(struct cn_msg) + sizeof(struct proc_event) + 4)

/* See comment above; we test our assumption about sizeof struct cn_msg here. */
static inline struct cn_msg *buffer_to_cn_msg(__u8 *buffer)
{
BUILD_BUG_ON(sizeof(struct cn_msg) != 20);
return (struct cn_msg *)(buffer + 4);
}

static atomic_t proc_event_num_listeners = ATOMIC_INIT(0);
static struct cb_id cn_proc_event_id = { CN_IDX_PROC, CN_VAL_PROC };
Expand All @@ -56,19 +68,19 @@ void proc_fork_connector(struct task_struct *task)
{
struct cn_msg *msg;
struct proc_event *ev;
__u8 buffer[CN_PROC_MSG_SIZE];
__u8 buffer[CN_PROC_MSG_SIZE] __aligned(8);
struct timespec ts;
struct task_struct *parent;

if (atomic_read(&proc_event_num_listeners) < 1)
return;

msg = (struct cn_msg *)buffer;
msg = buffer_to_cn_msg(buffer);
ev = (struct proc_event *)msg->data;
memset(&ev->event_data, 0, sizeof(ev->event_data));
get_seq(&msg->seq, &ev->cpu);
ktime_get_ts(&ts); /* get high res monotonic timestamp */
put_unaligned(timespec_to_ns(&ts), (__u64 *)&ev->timestamp_ns);
ev->timestamp_ns = timespec_to_ns(&ts);
ev->what = PROC_EVENT_FORK;
rcu_read_lock();
parent = rcu_dereference(task->real_parent);
Expand All @@ -91,17 +103,17 @@ void proc_exec_connector(struct task_struct *task)
struct cn_msg *msg;
struct proc_event *ev;
struct timespec ts;
__u8 buffer[CN_PROC_MSG_SIZE];
__u8 buffer[CN_PROC_MSG_SIZE] __aligned(8);

if (atomic_read(&proc_event_num_listeners) < 1)
return;

msg = (struct cn_msg *)buffer;
msg = buffer_to_cn_msg(buffer);
ev = (struct proc_event *)msg->data;
memset(&ev->event_data, 0, sizeof(ev->event_data));
get_seq(&msg->seq, &ev->cpu);
ktime_get_ts(&ts); /* get high res monotonic timestamp */
put_unaligned(timespec_to_ns(&ts), (__u64 *)&ev->timestamp_ns);
ev->timestamp_ns = timespec_to_ns(&ts);
ev->what = PROC_EVENT_EXEC;
ev->event_data.exec.process_pid = task->pid;
ev->event_data.exec.process_tgid = task->tgid;
Expand All @@ -117,14 +129,14 @@ void proc_id_connector(struct task_struct *task, int which_id)
{
struct cn_msg *msg;
struct proc_event *ev;
__u8 buffer[CN_PROC_MSG_SIZE];
__u8 buffer[CN_PROC_MSG_SIZE] __aligned(8);
struct timespec ts;
const struct cred *cred;

if (atomic_read(&proc_event_num_listeners) < 1)
return;

msg = (struct cn_msg *)buffer;
msg = buffer_to_cn_msg(buffer);
ev = (struct proc_event *)msg->data;
memset(&ev->event_data, 0, sizeof(ev->event_data));
ev->what = which_id;
Expand All @@ -145,7 +157,7 @@ void proc_id_connector(struct task_struct *task, int which_id)
rcu_read_unlock();
get_seq(&msg->seq, &ev->cpu);
ktime_get_ts(&ts); /* get high res monotonic timestamp */
put_unaligned(timespec_to_ns(&ts), (__u64 *)&ev->timestamp_ns);
ev->timestamp_ns = timespec_to_ns(&ts);

memcpy(&msg->id, &cn_proc_event_id, sizeof(msg->id));
msg->ack = 0; /* not used */
Expand All @@ -159,17 +171,17 @@ void proc_sid_connector(struct task_struct *task)
struct cn_msg *msg;
struct proc_event *ev;
struct timespec ts;
__u8 buffer[CN_PROC_MSG_SIZE];
__u8 buffer[CN_PROC_MSG_SIZE] __aligned(8);

if (atomic_read(&proc_event_num_listeners) < 1)
return;

msg = (struct cn_msg *)buffer;
msg = buffer_to_cn_msg(buffer);
ev = (struct proc_event *)msg->data;
memset(&ev->event_data, 0, sizeof(ev->event_data));
get_seq(&msg->seq, &ev->cpu);
ktime_get_ts(&ts); /* get high res monotonic timestamp */
put_unaligned(timespec_to_ns(&ts), (__u64 *)&ev->timestamp_ns);
ev->timestamp_ns = timespec_to_ns(&ts);
ev->what = PROC_EVENT_SID;
ev->event_data.sid.process_pid = task->pid;
ev->event_data.sid.process_tgid = task->tgid;
Expand All @@ -186,17 +198,17 @@ void proc_ptrace_connector(struct task_struct *task, int ptrace_id)
struct cn_msg *msg;
struct proc_event *ev;
struct timespec ts;
__u8 buffer[CN_PROC_MSG_SIZE];
__u8 buffer[CN_PROC_MSG_SIZE] __aligned(8);

if (atomic_read(&proc_event_num_listeners) < 1)
return;

msg = (struct cn_msg *)buffer;
msg = buffer_to_cn_msg(buffer);
ev = (struct proc_event *)msg->data;
memset(&ev->event_data, 0, sizeof(ev->event_data));
get_seq(&msg->seq, &ev->cpu);
ktime_get_ts(&ts); /* get high res monotonic timestamp */
put_unaligned(timespec_to_ns(&ts), (__u64 *)&ev->timestamp_ns);
ev->timestamp_ns = timespec_to_ns(&ts);
ev->what = PROC_EVENT_PTRACE;
ev->event_data.ptrace.process_pid = task->pid;
ev->event_data.ptrace.process_tgid = task->tgid;
Expand All @@ -221,17 +233,17 @@ void proc_comm_connector(struct task_struct *task)
struct cn_msg *msg;
struct proc_event *ev;
struct timespec ts;
__u8 buffer[CN_PROC_MSG_SIZE];
__u8 buffer[CN_PROC_MSG_SIZE] __aligned(8);

if (atomic_read(&proc_event_num_listeners) < 1)
return;

msg = (struct cn_msg *)buffer;
msg = buffer_to_cn_msg(buffer);
ev = (struct proc_event *)msg->data;
memset(&ev->event_data, 0, sizeof(ev->event_data));
get_seq(&msg->seq, &ev->cpu);
ktime_get_ts(&ts); /* get high res monotonic timestamp */
put_unaligned(timespec_to_ns(&ts), (__u64 *)&ev->timestamp_ns);
ev->timestamp_ns = timespec_to_ns(&ts);
ev->what = PROC_EVENT_COMM;
ev->event_data.comm.process_pid = task->pid;
ev->event_data.comm.process_tgid = task->tgid;
Expand All @@ -248,18 +260,18 @@ void proc_coredump_connector(struct task_struct *task)
{
struct cn_msg *msg;
struct proc_event *ev;
__u8 buffer[CN_PROC_MSG_SIZE];
__u8 buffer[CN_PROC_MSG_SIZE] __aligned(8);
struct timespec ts;

if (atomic_read(&proc_event_num_listeners) < 1)
return;

msg = (struct cn_msg *)buffer;
msg = buffer_to_cn_msg(buffer);
ev = (struct proc_event *)msg->data;
memset(&ev->event_data, 0, sizeof(ev->event_data));
get_seq(&msg->seq, &ev->cpu);
ktime_get_ts(&ts); /* get high res monotonic timestamp */
put_unaligned(timespec_to_ns(&ts), (__u64 *)&ev->timestamp_ns);
ev->timestamp_ns = timespec_to_ns(&ts);
ev->what = PROC_EVENT_COREDUMP;
ev->event_data.coredump.process_pid = task->pid;
ev->event_data.coredump.process_tgid = task->tgid;
Expand All @@ -275,18 +287,18 @@ void proc_exit_connector(struct task_struct *task)
{
struct cn_msg *msg;
struct proc_event *ev;
__u8 buffer[CN_PROC_MSG_SIZE];
__u8 buffer[CN_PROC_MSG_SIZE] __aligned(8);
struct timespec ts;

if (atomic_read(&proc_event_num_listeners) < 1)
return;

msg = (struct cn_msg *)buffer;
msg = buffer_to_cn_msg(buffer);
ev = (struct proc_event *)msg->data;
memset(&ev->event_data, 0, sizeof(ev->event_data));
get_seq(&msg->seq, &ev->cpu);
ktime_get_ts(&ts); /* get high res monotonic timestamp */
put_unaligned(timespec_to_ns(&ts), (__u64 *)&ev->timestamp_ns);
ev->timestamp_ns = timespec_to_ns(&ts);
ev->what = PROC_EVENT_EXIT;
ev->event_data.exit.process_pid = task->pid;
ev->event_data.exit.process_tgid = task->tgid;
Expand All @@ -312,18 +324,18 @@ static void cn_proc_ack(int err, int rcvd_seq, int rcvd_ack)
{
struct cn_msg *msg;
struct proc_event *ev;
__u8 buffer[CN_PROC_MSG_SIZE];
__u8 buffer[CN_PROC_MSG_SIZE] __aligned(8);
struct timespec ts;

if (atomic_read(&proc_event_num_listeners) < 1)
return;

msg = (struct cn_msg *)buffer;
msg = buffer_to_cn_msg(buffer);
ev = (struct proc_event *)msg->data;
memset(&ev->event_data, 0, sizeof(ev->event_data));
msg->seq = rcvd_seq;
ktime_get_ts(&ts); /* get high res monotonic timestamp */
put_unaligned(timespec_to_ns(&ts), (__u64 *)&ev->timestamp_ns);
ev->timestamp_ns = timespec_to_ns(&ts);
ev->cpu = -1;
ev->what = PROC_EVENT_NONE;
ev->event_data.ack.err = err;
Expand Down

0 comments on commit 1ca1a4c

Please sign in to comment.