Re: [UPDATED PATCH] fix memory corruption from misinterpretedbad_inode_ops return values

From: Roman Zippel
Date: Sat Jan 06 2007 - 21:16:12 EST


Hi,

On Thu, 4 Jan 2007, Al Viro wrote:

> PS: what would be the sane strategy for timer series merge, BTW?

PPS: I still don't like it. It fixes a rather theoretical problem with
absolutely no practical relevance.
PPPS: type safety is also possible with container_of(), the prototype
patch below demonstrates how to check that the signature matches and
additionally it doesn't require to convert everything at once. More
sophisticated checks can be done by putting this information into a
separate section, from where it can be extracted at compile/run time.

bye, Roman

---
include/linux/timer.h | 24 ++++++++++++++++++++++++
kernel/timer.c | 18 +++++++++++++++++-
kernel/workqueue.c | 24 ++++++++++--------------
3 files changed, 51 insertions(+), 15 deletions(-)

Index: linux-2.6-git/include/linux/timer.h
===================================================================
--- linux-2.6-git.orig/include/linux/timer.h 2007-01-06 20:45:02.000000000 +0100
+++ linux-2.6-git/include/linux/timer.h 2007-01-06 21:00:07.000000000 +0100
@@ -13,16 +13,40 @@ struct timer_list {

void (*function)(unsigned long);
unsigned long data;
+ unsigned long _sig;

struct tvec_t_base_s *base;
};

+#define __timer_sig(type, member) ((offsetof(type, member) << 16) | sizeof(type))
+
+#define __TIMER_OLD_SIG (0xa5005a)
+
+typedef void timer_cb_t(struct timer_list *);
+
+extern void __timer_init(struct timer_list *t, timer_cb_t *func, long sig);
+
+#define timer_init(ptr, member, func) \
+ __timer_init(&(ptr)->member, func, \
+ __timer_sig(typeof(*ptr), member))
+
+#define timer_of(ptr, type, member) ({ \
+ const struct timer_list *_t = (ptr); \
+ if (_t->_sig != __timer_sig(type, member)) { \
+ WARN_ON(_t->_sig != -__timer_sig(type, member));\
+ return; \
+ } \
+ container_of(ptr, type, member); \
+})
+
+
extern struct tvec_t_base_s boot_tvec_bases;

#define TIMER_INITIALIZER(_function, _expires, _data) { \
.function = (_function), \
.expires = (_expires), \
.data = (_data), \
+ ._sig = __TIMER_OLD_SIG, \
.base = &boot_tvec_bases, \
}

Index: linux-2.6-git/kernel/timer.c
===================================================================
--- linux-2.6-git.orig/kernel/timer.c 2007-01-06 20:45:02.000000000 +0100
+++ linux-2.6-git/kernel/timer.c 2007-01-06 21:00:07.000000000 +0100
@@ -226,6 +226,11 @@ static void internal_add_timer(tvec_base
unsigned long idx = expires - base->timer_jiffies;
struct list_head *vec;

+ if (!timer->_sig) {
+ WARN_ON(1);
+ timer->_sig = __TIMER_OLD_SIG;
+ }
+
if (idx < TVR_SIZE) {
int i = expires & TVR_MASK;
vec = base->tv1.vec + i;
@@ -271,11 +276,22 @@ static void internal_add_timer(tvec_base
*/
void fastcall init_timer(struct timer_list *timer)
{
+ timer->_sig = __TIMER_OLD_SIG;
timer->entry.next = NULL;
timer->base = __raw_get_cpu_var(tvec_bases);
}
EXPORT_SYMBOL(init_timer);

+extern void __timer_init(struct timer_list *t, timer_cb_t *func, long sig)
+{
+ t->function = (void *)func;
+ t->_sig = -sig;
+ func(t);
+ t->_sig = sig;
+ t->entry.next = NULL;
+ t->base = __raw_get_cpu_var(tvec_bases);
+}
+
static inline void detach_timer(struct timer_list *timer,
int clear_pending)
{
@@ -567,7 +583,7 @@ static inline void __run_timers(tvec_bas

timer = list_entry(head->next,struct timer_list,entry);
fn = timer->function;
- data = timer->data;
+ data = timer->_sig == __TIMER_OLD_SIG ? timer->data : (unsigned long)timer;

set_running_timer(base, timer);
detach_timer(timer, 1);
Index: linux-2.6-git/kernel/workqueue.c
===================================================================
--- linux-2.6-git.orig/kernel/workqueue.c 2007-01-06 19:51:40.000000000 +0100
+++ linux-2.6-git/kernel/workqueue.c 2007-01-06 21:02:59.000000000 +0100
@@ -218,9 +218,9 @@ int fastcall queue_work(struct workqueue
}
EXPORT_SYMBOL_GPL(queue_work);

-static void delayed_work_timer_fn(unsigned long __data)
+static void delayed_work_timer_fn(struct timer_list *timer)
{
- struct delayed_work *dwork = (struct delayed_work *)__data;
+ struct delayed_work *dwork = timer_of(timer, struct delayed_work, timer);
struct workqueue_struct *wq = get_wq_data(&dwork->work);
int cpu = smp_processor_id();

@@ -242,22 +242,20 @@ int fastcall queue_delayed_work(struct w
struct delayed_work *dwork, unsigned long delay)
{
int ret = 0;
- struct timer_list *timer = &dwork->timer;
struct work_struct *work = &dwork->work;

if (delay == 0)
return queue_work(wq, work);

if (!test_and_set_bit(WORK_STRUCT_PENDING, work_data_bits(work))) {
- BUG_ON(timer_pending(timer));
+ BUG_ON(timer_pending(&dwork->timer));
BUG_ON(!list_empty(&work->entry));

/* This stores wq for the moment, for the timer_fn */
set_wq_data(work, wq);
- timer->expires = jiffies + delay;
- timer->data = (unsigned long)dwork;
- timer->function = delayed_work_timer_fn;
- add_timer(timer);
+ timer_init(dwork, timer, delayed_work_timer_fn);
+ dwork->timer.expires = jiffies + delay;
+ add_timer(&dwork->timer);
ret = 1;
}
return ret;
@@ -277,19 +275,17 @@ int queue_delayed_work_on(int cpu, struc
struct delayed_work *dwork, unsigned long delay)
{
int ret = 0;
- struct timer_list *timer = &dwork->timer;
struct work_struct *work = &dwork->work;

if (!test_and_set_bit(WORK_STRUCT_PENDING, work_data_bits(work))) {
- BUG_ON(timer_pending(timer));
+ BUG_ON(timer_pending(&dwork->timer));
BUG_ON(!list_empty(&work->entry));

/* This stores wq for the moment, for the timer_fn */
set_wq_data(work, wq);
- timer->expires = jiffies + delay;
- timer->data = (unsigned long)dwork;
- timer->function = delayed_work_timer_fn;
- add_timer_on(timer, cpu);
+ timer_init(dwork, timer, delayed_work_timer_fn);
+ dwork->timer.expires = jiffies + delay;
+ add_timer_on(&dwork->timer, cpu);
ret = 1;
}
return ret;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/