linux-kernel - Re: [Cocci] [PATCH] coccicheck: add a test for repeat copy_from

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 11 Jan 2017 21:44:00 +0800
From:   Pengfei Wang <wpengfeinudt@...il.com>
To:     Julia Lawall <julia.lawall@...6.fr>
Cc:     Vaishali Thakkar <vaishali.thakkar@...cle.com>,
        Kees Cook <keescook@...omium.org>,
        Vaishali Thakkar <vthakkar1994@...il.com>,
        linux-kernel@...r.kernel.org, Michal Marek <mmarek@...e.com>,
        cocci@...teme.lip6.fr
Subject: Re: [Cocci] [PATCH] coccicheck: add a test for repeat copy_from_user


> 在 2017年1月11日，下午2:12，Julia Lawall <julia.lawall@...6.fr> 写道：
> 
> I looked at the get_user part of the original script.  It looks like most
> of the complexity is to deal with the possibility of the src location
> being expressed in two different ways between the two calls.  

Yes, in addition to pointer alias and the “field-whole” double-fetch type 
as we mentioned previously, we also need to take into consideration of 
the explicit data type conversion of the src pointer, such as:
get_user(dst, src)
…
get_user(dst, (int*)src)

Also the embedded computation at the argument position, such as 
get_user(dst, ++src) , get_user(dst, align(src)), or get_user(dst, src[i]),
which could cause false positives. Loops also cause false positives.


> Even if this
> happens in practice only for get_user, it would seem that it could happen
> for copy_from_user as well.  So I think we could just throw both get_user
> and copy_from_user into the same rule?
> 
Agreed.

> I'm also not sure to understand why there are cases for things like
> 
> get_user(exp1, src->f1)
> ...
> get_user(exp2,src)
> 
> Can this happen?  The types seem wrong.
I think it is unreasonable. It doesn’t work in practice. It exists in my script 
because I combined different situations with disjunction but forgot to remove 
the infeasible ones. Please remove it. A practical one should be:
get_user(exp1, src->f1)
…
copy_from_user(exp2, src ,size)

> Likewise, I see the need to take into account a second argument of src++,
> but not the need to take into account a second argument of src+4.  Either
> there is src+4 in both calls or the addresses involved are just different.

src++ or src+4 are used when handling long messages byte by byte or word 
by word by means of a loop. I paid attention to these because they cause 
false positives as src pointers have changed for the double fetches. I remember 
both of these two situations when examining the source code but I cannot 
guarantee the src+4 situation exists as I don’t have an example in hand now. I suggest 
we focus on the src++ for now. If src+4 cause any false positives, we’ll add it, too.

Regards
Pengfei

> 
> Perhaps I'm missing something, though.
> 
> julia
>