Re: [PATCH] Fix for rtc.c non-atomic access to rtc_status

From: Manfred Spraul (manfreds@colorfullife.com)
Date: Sat Apr 22 2000 - 08:40:07 EDT

  • Next message: bert hubert: "Re: namei() query"

    Cesar Eduardo Barros wrote:
    >
    > (not on the list, please CC: replies to me)
    >
    > Using atomic_* seems to cause a false sense of security in some people.
    > atomic_set(...,atomic_read(...)) is not atomic, and rtc.c did it everywhere.
    > Also, testing for a bit and later enabling it (like in rtc_open) causes races
    > (two proccesses in SMP could open the rtc at the same time if they got the
    > right timing). I patched it to use a spinlock instead.
    >

    rtc_release, rtc_open and rtc_ioctl are protected by lock_kernel(), so
    they are protected from SMP races.

    >
    > if (rtc_async_queue)
    > + /* Is this OK on interrupt? */
    > kill_fasync (rtc_async_queue, SIGIO, POLL_IN);
    >

    yes.

    > @@ -699,8 +717,13 @@
    > {
    > /* interrupts and maybe timer disabled at this point by rtc_release */
    >
    > - if (atomic_read(&rtc_status) & RTC_TIMER_ON)
    > + /* FIXME: how rtc_open here could be avoided? */

    the magic of lock_kernel() helps:
    rtc_exit() and rtc_open() both run with the big kernel lock.

    I'm far more concerned about the xchg() in rtc_read:
    that xchg() and rtc_interrupt() aren't synchronized, the rest could be
    fixed with test_and_set(), atomic_mask(),... 

    --
	Manfred

    -
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/

    This archive was generated by hypermail 2b29 : Sat Apr 22 2000 - 08:42:59 EDT