Skip to content

Commit 1ca1a4c

Browse files
cmetcalf-tileradavem330
authored andcommitted
connector: improved unaligned access error fix
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 <cmetcalf@tilera.com> Signed-off-by: David S. Miller <davem@davemloft.net>
1 parent 2abc2f0 commit 1ca1a4c

File tree

1 file changed

+42
-30
lines changed

1 file changed

+42
-30
lines changed

drivers/connector/cn_proc.c

Lines changed: 42 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,23 @@
3232
#include <linux/atomic.h>
3333
#include <linux/pid_namespace.h>
3434

35-
#include <asm/unaligned.h>
36-
3735
#include <linux/cn_proc.h>
3836

39-
#define CN_PROC_MSG_SIZE (sizeof(struct cn_msg) + sizeof(struct proc_event))
37+
/*
38+
* Size of a cn_msg followed by a proc_event structure. Since the
39+
* sizeof struct cn_msg is a multiple of 4 bytes, but not 8 bytes, we
40+
* add one 4-byte word to the size here, and then start the actual
41+
* cn_msg structure 4 bytes into the stack buffer. The result is that
42+
* the immediately following proc_event structure is aligned to 8 bytes.
43+
*/
44+
#define CN_PROC_MSG_SIZE (sizeof(struct cn_msg) + sizeof(struct proc_event) + 4)
45+
46+
/* See comment above; we test our assumption about sizeof struct cn_msg here. */
47+
static inline struct cn_msg *buffer_to_cn_msg(__u8 *buffer)
48+
{
49+
BUILD_BUG_ON(sizeof(struct cn_msg) != 20);
50+
return (struct cn_msg *)(buffer + 4);
51+
}
4052

4153
static atomic_t proc_event_num_listeners = ATOMIC_INIT(0);
4254
static struct cb_id cn_proc_event_id = { CN_IDX_PROC, CN_VAL_PROC };
@@ -56,19 +68,19 @@ void proc_fork_connector(struct task_struct *task)
5668
{
5769
struct cn_msg *msg;
5870
struct proc_event *ev;
59-
__u8 buffer[CN_PROC_MSG_SIZE];
71+
__u8 buffer[CN_PROC_MSG_SIZE] __aligned(8);
6072
struct timespec ts;
6173
struct task_struct *parent;
6274

6375
if (atomic_read(&proc_event_num_listeners) < 1)
6476
return;
6577

66-
msg = (struct cn_msg *)buffer;
78+
msg = buffer_to_cn_msg(buffer);
6779
ev = (struct proc_event *)msg->data;
6880
memset(&ev->event_data, 0, sizeof(ev->event_data));
6981
get_seq(&msg->seq, &ev->cpu);
7082
ktime_get_ts(&ts); /* get high res monotonic timestamp */
71-
put_unaligned(timespec_to_ns(&ts), (__u64 *)&ev->timestamp_ns);
83+
ev->timestamp_ns = timespec_to_ns(&ts);
7284
ev->what = PROC_EVENT_FORK;
7385
rcu_read_lock();
7486
parent = rcu_dereference(task->real_parent);
@@ -91,17 +103,17 @@ void proc_exec_connector(struct task_struct *task)
91103
struct cn_msg *msg;
92104
struct proc_event *ev;
93105
struct timespec ts;
94-
__u8 buffer[CN_PROC_MSG_SIZE];
106+
__u8 buffer[CN_PROC_MSG_SIZE] __aligned(8);
95107

96108
if (atomic_read(&proc_event_num_listeners) < 1)
97109
return;
98110

99-
msg = (struct cn_msg *)buffer;
111+
msg = buffer_to_cn_msg(buffer);
100112
ev = (struct proc_event *)msg->data;
101113
memset(&ev->event_data, 0, sizeof(ev->event_data));
102114
get_seq(&msg->seq, &ev->cpu);
103115
ktime_get_ts(&ts); /* get high res monotonic timestamp */
104-
put_unaligned(timespec_to_ns(&ts), (__u64 *)&ev->timestamp_ns);
116+
ev->timestamp_ns = timespec_to_ns(&ts);
105117
ev->what = PROC_EVENT_EXEC;
106118
ev->event_data.exec.process_pid = task->pid;
107119
ev->event_data.exec.process_tgid = task->tgid;
@@ -117,14 +129,14 @@ void proc_id_connector(struct task_struct *task, int which_id)
117129
{
118130
struct cn_msg *msg;
119131
struct proc_event *ev;
120-
__u8 buffer[CN_PROC_MSG_SIZE];
132+
__u8 buffer[CN_PROC_MSG_SIZE] __aligned(8);
121133
struct timespec ts;
122134
const struct cred *cred;
123135

124136
if (atomic_read(&proc_event_num_listeners) < 1)
125137
return;
126138

127-
msg = (struct cn_msg *)buffer;
139+
msg = buffer_to_cn_msg(buffer);
128140
ev = (struct proc_event *)msg->data;
129141
memset(&ev->event_data, 0, sizeof(ev->event_data));
130142
ev->what = which_id;
@@ -145,7 +157,7 @@ void proc_id_connector(struct task_struct *task, int which_id)
145157
rcu_read_unlock();
146158
get_seq(&msg->seq, &ev->cpu);
147159
ktime_get_ts(&ts); /* get high res monotonic timestamp */
148-
put_unaligned(timespec_to_ns(&ts), (__u64 *)&ev->timestamp_ns);
160+
ev->timestamp_ns = timespec_to_ns(&ts);
149161

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

164176
if (atomic_read(&proc_event_num_listeners) < 1)
165177
return;
166178

167-
msg = (struct cn_msg *)buffer;
179+
msg = buffer_to_cn_msg(buffer);
168180
ev = (struct proc_event *)msg->data;
169181
memset(&ev->event_data, 0, sizeof(ev->event_data));
170182
get_seq(&msg->seq, &ev->cpu);
171183
ktime_get_ts(&ts); /* get high res monotonic timestamp */
172-
put_unaligned(timespec_to_ns(&ts), (__u64 *)&ev->timestamp_ns);
184+
ev->timestamp_ns = timespec_to_ns(&ts);
173185
ev->what = PROC_EVENT_SID;
174186
ev->event_data.sid.process_pid = task->pid;
175187
ev->event_data.sid.process_tgid = task->tgid;
@@ -186,17 +198,17 @@ void proc_ptrace_connector(struct task_struct *task, int ptrace_id)
186198
struct cn_msg *msg;
187199
struct proc_event *ev;
188200
struct timespec ts;
189-
__u8 buffer[CN_PROC_MSG_SIZE];
201+
__u8 buffer[CN_PROC_MSG_SIZE] __aligned(8);
190202

191203
if (atomic_read(&proc_event_num_listeners) < 1)
192204
return;
193205

194-
msg = (struct cn_msg *)buffer;
206+
msg = buffer_to_cn_msg(buffer);
195207
ev = (struct proc_event *)msg->data;
196208
memset(&ev->event_data, 0, sizeof(ev->event_data));
197209
get_seq(&msg->seq, &ev->cpu);
198210
ktime_get_ts(&ts); /* get high res monotonic timestamp */
199-
put_unaligned(timespec_to_ns(&ts), (__u64 *)&ev->timestamp_ns);
211+
ev->timestamp_ns = timespec_to_ns(&ts);
200212
ev->what = PROC_EVENT_PTRACE;
201213
ev->event_data.ptrace.process_pid = task->pid;
202214
ev->event_data.ptrace.process_tgid = task->tgid;
@@ -221,17 +233,17 @@ void proc_comm_connector(struct task_struct *task)
221233
struct cn_msg *msg;
222234
struct proc_event *ev;
223235
struct timespec ts;
224-
__u8 buffer[CN_PROC_MSG_SIZE];
236+
__u8 buffer[CN_PROC_MSG_SIZE] __aligned(8);
225237

226238
if (atomic_read(&proc_event_num_listeners) < 1)
227239
return;
228240

229-
msg = (struct cn_msg *)buffer;
241+
msg = buffer_to_cn_msg(buffer);
230242
ev = (struct proc_event *)msg->data;
231243
memset(&ev->event_data, 0, sizeof(ev->event_data));
232244
get_seq(&msg->seq, &ev->cpu);
233245
ktime_get_ts(&ts); /* get high res monotonic timestamp */
234-
put_unaligned(timespec_to_ns(&ts), (__u64 *)&ev->timestamp_ns);
246+
ev->timestamp_ns = timespec_to_ns(&ts);
235247
ev->what = PROC_EVENT_COMM;
236248
ev->event_data.comm.process_pid = task->pid;
237249
ev->event_data.comm.process_tgid = task->tgid;
@@ -248,18 +260,18 @@ void proc_coredump_connector(struct task_struct *task)
248260
{
249261
struct cn_msg *msg;
250262
struct proc_event *ev;
251-
__u8 buffer[CN_PROC_MSG_SIZE];
263+
__u8 buffer[CN_PROC_MSG_SIZE] __aligned(8);
252264
struct timespec ts;
253265

254266
if (atomic_read(&proc_event_num_listeners) < 1)
255267
return;
256268

257-
msg = (struct cn_msg *)buffer;
269+
msg = buffer_to_cn_msg(buffer);
258270
ev = (struct proc_event *)msg->data;
259271
memset(&ev->event_data, 0, sizeof(ev->event_data));
260272
get_seq(&msg->seq, &ev->cpu);
261273
ktime_get_ts(&ts); /* get high res monotonic timestamp */
262-
put_unaligned(timespec_to_ns(&ts), (__u64 *)&ev->timestamp_ns);
274+
ev->timestamp_ns = timespec_to_ns(&ts);
263275
ev->what = PROC_EVENT_COREDUMP;
264276
ev->event_data.coredump.process_pid = task->pid;
265277
ev->event_data.coredump.process_tgid = task->tgid;
@@ -275,18 +287,18 @@ void proc_exit_connector(struct task_struct *task)
275287
{
276288
struct cn_msg *msg;
277289
struct proc_event *ev;
278-
__u8 buffer[CN_PROC_MSG_SIZE];
290+
__u8 buffer[CN_PROC_MSG_SIZE] __aligned(8);
279291
struct timespec ts;
280292

281293
if (atomic_read(&proc_event_num_listeners) < 1)
282294
return;
283295

284-
msg = (struct cn_msg *)buffer;
296+
msg = buffer_to_cn_msg(buffer);
285297
ev = (struct proc_event *)msg->data;
286298
memset(&ev->event_data, 0, sizeof(ev->event_data));
287299
get_seq(&msg->seq, &ev->cpu);
288300
ktime_get_ts(&ts); /* get high res monotonic timestamp */
289-
put_unaligned(timespec_to_ns(&ts), (__u64 *)&ev->timestamp_ns);
301+
ev->timestamp_ns = timespec_to_ns(&ts);
290302
ev->what = PROC_EVENT_EXIT;
291303
ev->event_data.exit.process_pid = task->pid;
292304
ev->event_data.exit.process_tgid = task->tgid;
@@ -312,18 +324,18 @@ static void cn_proc_ack(int err, int rcvd_seq, int rcvd_ack)
312324
{
313325
struct cn_msg *msg;
314326
struct proc_event *ev;
315-
__u8 buffer[CN_PROC_MSG_SIZE];
327+
__u8 buffer[CN_PROC_MSG_SIZE] __aligned(8);
316328
struct timespec ts;
317329

318330
if (atomic_read(&proc_event_num_listeners) < 1)
319331
return;
320332

321-
msg = (struct cn_msg *)buffer;
333+
msg = buffer_to_cn_msg(buffer);
322334
ev = (struct proc_event *)msg->data;
323335
memset(&ev->event_data, 0, sizeof(ev->event_data));
324336
msg->seq = rcvd_seq;
325337
ktime_get_ts(&ts); /* get high res monotonic timestamp */
326-
put_unaligned(timespec_to_ns(&ts), (__u64 *)&ev->timestamp_ns);
338+
ev->timestamp_ns = timespec_to_ns(&ts);
327339
ev->cpu = -1;
328340
ev->what = PROC_EVENT_NONE;
329341
ev->event_data.ack.err = err;

0 commit comments

Comments
 (0)