lkml.org 
[lkml]   [2007]   [Nov]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] debug_check_no_locks_freed: fix in_range() checks
    On Nov 24, 2007 11:53 AM, Oleg Nesterov <oleg@tv-sign.ru> wrote:
    > Torsten, could you ack/nack this patch?

    From looking at the code I would ack it.
    I will reapply agk-dm-dm-crypt-move-bio-submission-to-thread.patch and
    this patch and boot several times, but as the message was not
    triggered on every boot, this can't prove anything.

    But if it happens again, I will notify you.

    Torsten

    > 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 <oleg@tv-sign.ru>
    >
    > --- 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 majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2007-11-24 13:21    [W:0.030 / U:1.632 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site