Skip to content

Commit ee7751b

Browse files
beaubelgraverostedt
authored andcommitted
tracing/user_events: Use long vs int for atomic bit ops
Each event stores a int to track which bit to set/clear when enablement changes. On big endian 64-bit configurations, it's possible this could cause memory corruption when it's used for atomic bit operations. Use unsigned long for enablement values to ensure any possible corruption cannot occur. Downcast to int after mask for the bit target. Link: https://lore.kernel.org/all/6f758683-4e5e-41c3-9b05-9efc703e827c@kili.mountain/ Link: https://lore.kernel.org/linux-trace-kernel/20230505205855.6407-1-beaub@linux.microsoft.com Fixes: dcb8177 ("tracing/user_events: Add ioctl for disabling addresses") Reported-by: Dan Carpenter <dan.carpenter@linaro.org> Signed-off-by: Beau Belgrave <beaub@linux.microsoft.com> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
1 parent 44c026a commit ee7751b

1 file changed

Lines changed: 8 additions & 7 deletions

File tree

kernel/trace/trace_events_user.c

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ struct user_event_enabler {
101101
unsigned long addr;
102102

103103
/* Track enable bit, flags, etc. Aligned for bitops. */
104-
unsigned int values;
104+
unsigned long values;
105105
};
106106

107107
/* Bits 0-5 are for the bit to update upon enable/disable (0-63 allowed) */
@@ -116,7 +116,9 @@ struct user_event_enabler {
116116
/* Only duplicate the bit value */
117117
#define ENABLE_VAL_DUP_MASK ENABLE_VAL_BIT_MASK
118118

119-
#define ENABLE_BITOPS(e) ((unsigned long *)&(e)->values)
119+
#define ENABLE_BITOPS(e) (&(e)->values)
120+
121+
#define ENABLE_BIT(e) ((int)((e)->values & ENABLE_VAL_BIT_MASK))
120122

121123
/* Used for asynchronous faulting in of pages */
122124
struct user_event_enabler_fault {
@@ -423,9 +425,9 @@ static int user_event_enabler_write(struct user_event_mm *mm,
423425

424426
/* Update bit atomically, user tracers must be atomic as well */
425427
if (enabler->event && enabler->event->status)
426-
set_bit(enabler->values & ENABLE_VAL_BIT_MASK, ptr);
428+
set_bit(ENABLE_BIT(enabler), ptr);
427429
else
428-
clear_bit(enabler->values & ENABLE_VAL_BIT_MASK, ptr);
430+
clear_bit(ENABLE_BIT(enabler), ptr);
429431

430432
kunmap_local(kaddr);
431433
unpin_user_pages_dirty_lock(&page, 1, true);
@@ -440,8 +442,7 @@ static bool user_event_enabler_exists(struct user_event_mm *mm,
440442
struct user_event_enabler *next;
441443

442444
list_for_each_entry_safe(enabler, next, &mm->enablers, link) {
443-
if (enabler->addr == uaddr &&
444-
(enabler->values & ENABLE_VAL_BIT_MASK) == bit)
445+
if (enabler->addr == uaddr && ENABLE_BIT(enabler) == bit)
445446
return true;
446447
}
447448

@@ -2272,7 +2273,7 @@ static long user_events_ioctl_unreg(unsigned long uarg)
22722273

22732274
list_for_each_entry_safe(enabler, next, &mm->enablers, link)
22742275
if (enabler->addr == reg.disable_addr &&
2275-
(enabler->values & ENABLE_VAL_BIT_MASK) == reg.disable_bit) {
2276+
ENABLE_BIT(enabler) == reg.disable_bit) {
22762277
set_bit(ENABLE_VAL_FREEING_BIT, ENABLE_BITOPS(enabler));
22772278

22782279
if (!test_bit(ENABLE_VAL_FAULTING_BIT, ENABLE_BITOPS(enabler)))

0 commit comments

Comments
 (0)