Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 25 Oct 2014 13:25:09 -0700
From:      John-Mark Gurney <jmg@funkthat.com>
To:        Konstantin Belousov <kostikbel@gmail.com>
Cc:        arch@freebsd.org
Subject:   Re: RfC: fueword(9) and casueword(9)
Message-ID:  <20141025202509.GX82214@funkthat.com>
In-Reply-To: <20141021094539.GA1877@kib.kiev.ua>
References:  <20141021094539.GA1877@kib.kiev.ua>

next in thread | previous in thread | raw e-mail | index | archive | help
Konstantin Belousov wrote this message on Tue, Oct 21, 2014 at 12:45 +0300:
> FreeBSD provides the fuword(9) family of functions to fetch a word from
> the userspace. Functions return the value read, or -1 on failure (i.e.
> when faulted on access). This KPI has flaw, which makes it impossible
> to distinguish -1 read from usermode vs. the fault. As John Baldwin
> pointed out, fuword(9) cannot be replaced by copyin(9), since fuword(9)
> is atomic for aligned data, while copyin(9) is typically implemented as
> byte copy.

We also need to define what a word is in the man page...  I assumed
that a word (fuword) was a 32bit word, but it's not on 64 bit arches,
it's a 64bit word...  if words were 32bit words on 64bit arches, fuword
would be safe (assuming reading an unsigned word), but that is not the
case...

Related to this is that it isn't defined if fubyte (returns an int)
reads a signed or unsigned byte.  If it reads an unsigned byte, then
it is safe, and we do not need an fuebyte version...  The same goes
w/ fuword16 (returns an int)...

> I wanted to fix this wart for long time, below is the prototyped patch,
> which adds fueword(9) family of functions.  They take the address of
> variable where to put the value read, and return 0 on success, -1 on
> failure.  In similar way, casueword(9) fixes casuword(9).
> 
> The tricky part of the patch are the changes to kern_umtx.c, where the
> logic of the loops in the lock acquire routines is delicate and care
> must be taken to not obliterate possible errors from the suspension
> check or signal test on loop retry.
> 
> I only implemented fueword(9) and casueword(9) for x86 and powerpc.
> The fuword(9) and casuword(9) are reimplemented as wrappers around
> e-variants.
> 
> For arm, mips and sparc, where I do not know or do not remember the
> assembler anymore, I made a hack to provide deficient fueword(9), which
> calls fuword(9) and thus still mixing -1 from userspace and fault. See
> NO_FUEWORD in machine/param.h; hopefully arch maintainers will fix the
> remaining places.
> 
> Some users of fuword(9) are still left, in particular in aio and dtrace.
> 
> Patch was only lightly tested on x86 for now.
> 
> Comments and fixes are welcomed.

I'll take a closer look at the patch soon...

-- 
  John-Mark Gurney				Voice: +1 415 225 5579

     "All that I will do, has been done, All that I have, has not."



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