On Wed, Jan 16, 2019 at 05:22:48PM +1200, Linus Torvalds wrote:
> On Wed, Jan 16, 2019 at 10:59 AM Luc Van Oostenryck
> <luc.vanoostenryck@xxxxxxxxx> wrote:
> > *) I've always considered __noderef as really meaning "must not be
> > dereferenced except by some special purpose operation which
> > effectively do the real deref". In the present case (percpu &
> > uaccess (on x86)), it's clear that the special operation is
> > the asm memory op but isn't this essentially incidental?
>
> I agree that it's somewhat incidental rather than anything hugely by design.
>
> That said, the *design* of __noderef was literally to make sure that
> the *compiler* never derefences things, and the "special thing" that
> does dereference was literally meant to be an inline asm. The
> "{get,put}_user()" case was exactly what __noderef was designed for.
>
> So it *is* very much by design,
OK, that's very clear.
I'm much more familiar with __iomem accesses which I kinda used
as reference.
> The good news is that the asm constraints aren't really all that
> architecture-specific. Yes, all the various register classes etc
> obviously are, but within the concept of "the compiler never
> dereferences it", the only asm constraints that matter are "m" and
> "p", I think.
>
> "m" means "memory", and "p" means "address". They are kind of the
> same, except for a gcc memory access ordering situation, I think.
Well, it's alas more complicated :(
The common constraints for memory operands are "m", "o" & "V".
"p" is different, it must be a memory *address*, so there is
no possible problem with noderef with it.
So far so good but, for example:
*) ARM also has "Q" as memory operand (while for ARM64, "Q" is a
memory address) as well as "Uv", "Uy" & "Uq"
*) PPC also has "wF", "wG", "wO", "es" "Q" & "Z"
*) S390 has "Q", "R", "S" & "T"
*) ...
That's really annoying, especially the ARM vs ARM64 "Q".
Of course, sparse could just take care of "m", "o" & "V" but
that seems really wrong.
ARM64 also use "Ump" for memory *address*, so simply scanning for
the charatcer "m" will gives false positives. Sparse would need to
known all constraints for all archs :(
> > Thinking a bit more about it and looking at some dumps I made:
> > It seems that most of these these 'noderef in asm operand'
> > warnings (99.5%) come from percpu operations (which have an API
> > using an lvalue and not a pointer, unlike the uaccess API).
> >
> > I begin to suspect that the warnings occuring with an uaccess
> > operation come from some other kind of error (possibly a missing
> > call to degenerate()).
>
> Hmm. The uaccess cases definitely pass the lvalue in some cases. Not
> the cases where we just end up doing a special "call" sequence (the
> normal get_user/put_user() case). But look closer: the cases where we
> actaully inline the access itself, we generate a "LKmode" lvalue with
> that "__m(addr)" thing, and the actual access is done by the inline
> asm.
>
> See __get_user_asm(), __get_user_asm_nozero(), __get_user_asm_ex() and
> __put_user_goto().
Yes, indeed I was misled by the relatively small number of these.
That said, asm input operands really miss a call to degenerate().
-- Luc
