Torsten, could you ack/nack this patch?
Torsten Kaiser wrote:
>
> static inline int in_range(const void *start, const void *addr, const void *end)
> {
> return addr >= start && addr <= end;
> }
> This will return true, if addr is in the range of start (including)
> to end (including).
>
> But debug_check_no_locks_freed() seems does:
> const void *mem_to = mem_from + mem_len
> -> mem_to is the last byte of the freed range, that fits in_range
> lock_from = (void *)hlock->instance;
> -> first byte of the lock
> lock_to = (void *)(hlock->instance + 1);
> -> first byte of the next lock, not last byte of the lock that is being checked!
>
> The test is:
> if (!in_range(mem_from, lock_from, mem_to) &&
> !in_range(mem_from, lock_to, mem_to))
> continue;
> So it tests, if the first byte of the lock is in the range that is freed ->OK
> And if the first byte of the *next* lock is in the range that is freed
> -> Not OK.
We can also simplify in_range checks, we need only 2 comparisons, not 4.
If the lock is not in memory range, it should be either at the left of range
or at the right.
Signed-off-by: Oleg Nesterov <[email protected]>
--- 24/kernel/lockdep.c~ 2007-11-09 12:57:31.000000000 +0300
+++ 24/kernel/lockdep.c 2007-11-24 13:32:52.000000000 +0300
@@ -3054,11 +3054,6 @@ void __init lockdep_info(void)
#endif
}
-static inline int in_range(const void *start, const void *addr, const void *end)
-{
- return addr >= start && addr <= end;
-}
-
static void
print_freed_lock_bug(struct task_struct *curr, const void *mem_from,
const void *mem_to, struct held_lock *hlock)
@@ -3080,6 +3075,13 @@ print_freed_lock_bug(struct task_struct
dump_stack();
}
+static inline int not_in_range(const void* mem_from, unsigned long mem_len,
+ const void* lock_from, unsigned long lock_len)
+{
+ return lock_from + lock_len <= mem_from ||
+ mem_from + mem_len <= lock_from;
+}
+
/*
* Called when kernel memory is freed (or unmapped), or if a lock
* is destroyed or reinitialized - this code checks whether there is
@@ -3087,7 +3089,6 @@ print_freed_lock_bug(struct task_struct
*/
void debug_check_no_locks_freed(const void *mem_from, unsigned long mem_len)
{
- const void *mem_to = mem_from + mem_len, *lock_from, *lock_to;
struct task_struct *curr = current;
struct held_lock *hlock;
unsigned long flags;
@@ -3100,14 +3101,11 @@ void debug_check_no_locks_freed(const vo
for (i = 0; i < curr->lockdep_depth; i++) {
hlock = curr->held_locks + i;
- lock_from = (void *)hlock->instance;
- lock_to = (void *)(hlock->instance + 1);
-
- if (!in_range(mem_from, lock_from, mem_to) &&
- !in_range(mem_from, lock_to, mem_to))
+ if (not_in_range(mem_from, mem_len, hlock->instance,
+ sizeof(*hlock->instance)))
continue;
- print_freed_lock_bug(curr, mem_from, mem_to, hlock);
+ print_freed_lock_bug(curr, mem_from, mem_from + mem_len, hlock);
break;
}
local_irq_restore(flags);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
