Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 8 Oct 2003 08:36:15 -0700 (PDT)
From:      Frank Mayhar <frank@exit.com>
To:        Pawel Jakub Dawidek <nick@garage.freebsd.pl>
Cc:        hsu@freebsd.org
Subject:   Re: Dynamic reads without locking.
Message-ID:  <200310081536.h98FaFa1087800@realtime.exit.com>
In-Reply-To: <20031008083059.GA520@garage.freebsd.pl>

next in thread | previous in thread | raw e-mail | index | archive | help
I read the thread hoping to see a succint response to this and so far I don't
see it.  Here goes...

Pawel Jakub Dawidek wrote:
> I'm wondering...
> Jeffrey Hsu was talking about this at BSDCon03.
> There is no need to lock data when we just made simple read, for example:
> 
> 	mtx_lock(&foo_mtx);
> 	foo = 5;
> 	mtx_unlock(&foo_mtx);
> but only:
> 	bar = foo;
> 
> IMHO this is quite dangerous.
> Let's see:
> 
> 	thread1			thread2
> 	mtx_lock(&foo_mtx);
> 	foo = data_from_user;
> 				bar = foo;
> 	foo &= MASK;
> 	mtx_unlock(&foo_mtx);
> 
> In this case we have really dangerous race if data from user are
> safe only when we made 'and' operation on them.
> OR of course we can just store wrong value in 'bar' and this could
> be case of different problems.

There are at least two different things going on here.  First off, in
general an unlocked read is generally only "safe" if the writes themselves
are atomic.  And I mean atomic _without_ using locks.  So the locked update
of "foo" up there should really be:

	thread1				thread2
	foo = (data_from_user & MASK)	bar = foo

So you see there is a simple race here.  As long as the write to foo in
thread1 is atomic by nature (which really depends on the architecture,
but that's a different thread), either thread2 will end up with a stale
value or it will end up with the new value.  Either way, it will be a
_valid_ value.

> So I'm not sure now if I understand everything well. We can't just say
> 'We never split such writes. We always do: foo = (data_from_user & MASK)',
> because author of some 3rd party kernel module will be sure that when
> he locks writes to some variable this operation is safe and he could
> split such writes and in kernel could be dynamic read without lock.

The other thing is that the unlocked reads about which I assume Jeffrey
Hsu was speaking can only be used in very specific cases, where one has
control over both the write and the read.  If you have to handle unmodified
third-party modules, you have no choice but to do locking, for the reason
you indicate.  On the other hand, you can indeed make such a rule as you
describe:  For this global datum, we always either write or read it in a
single operation, we never do a write/read/modify/write.  Hey, if it's
your kernel, you can make the rules you want to make!  But it's better
to not allow third-party modules to use those global data.

In fact, it could be that the compiler may optimize your example into
a single operation.  The way it is written, it's just bad coding practice,
at least in this case.

I don't really see that there's much about which to disagree, here.  Jeffrey
is right in at least certain well-defined cases.  In the general case, though,
you have to use some kind of explicit serialization.
-- 
Frank Mayhar frank@exit.com	http://www.exit.com/
Exit Consulting                 http://www.gpsclock.com/
                                http://www.exit.com/blog/frank/



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200310081536.h98FaFa1087800>