Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1205724 > unrolled thread
| Started by | yalin wang <yalin.wang2010@gmail.com> |
|---|---|
| First post | 2015-08-12 11:10 +0200 |
| Last post | 2015-08-17 06:20 +0200 |
| Articles | 6 — 2 participants |
Back to article view | Back to linux.kernel
[x86] copy_from{to}_user question yalin wang <yalin.wang2010@gmail.com> - 2015-08-12 11:10 +0200
Re: [x86] copy_from{to}_user question Borislav Petkov <bp@suse.de> - 2015-08-12 12:10 +0200
Re: [x86] copy_from{to}_user question yalin wang <yalin.wang2010@gmail.com> - 2015-08-13 12:10 +0200
Re: [x86] copy_from{to}_user question Borislav Petkov <bp@suse.de> - 2015-08-13 18:50 +0200
Re: [x86] copy_from{to}_user question yalin wang <yalin.wang2010@gmail.com> - 2015-08-17 05:30 +0200
Re: [x86] copy_from{to}_user question Borislav Petkov <bp@suse.de> - 2015-08-17 06:20 +0200
| From | yalin wang <yalin.wang2010@gmail.com> |
|---|---|
| Date | 2015-08-12 11:10 +0200 |
| Subject | [x86] copy_from{to}_user question |
| Message-ID | <pWAcO-8hY-1@gated-at.bofh.it> |
hi x86 maintainers,
i have a question about copy_from{to}_user() function,
i find on other platforms like arm/ arm64 /hexagon,
all copy_from{to}_user function only check source address for
copy_from and only check to address for copy_to user function,
never check both source and dest together,
but on x86 platform, i see copy_from{to}_user use a generic function
named copy_user_generic_unrolled() in arch/x86/lib/copy_user_64.S,
it check source and dest address no matter it is copy_from user or
copy_to_user , is it correct?
for copy_from_user i think only need check source address is enough,
if check both address, may hide some kernel BUG, if the kernel address
is not valid, because the fix up code will fix it and kernel will
not panic in this situation.
another problems is that in ./fs/proc/kcore.c ,
read_kcore() function:
if (kern_addr_valid(start)) {
unsigned long n;
n = copy_to_user(buffer, (char *)start, tsz);
/*
¦* We cannot distinguish between fault on source
¦* and fault on destination. When this happens
¦* we clear too and hope it will trigger the
¦* EFAULT again.
¦*/
if (n) {
if (clear_user(buffer + tsz - n,
n))
return -EFAULT;
}
} else {
if (clear_user(buffer, tsz))
return -EFAULT;
}
it relies on copy_to_user() can fault on both user and kernel address,
it is not true on arm / arm64 /hexgon platforms, maybe some other platforms,
i don’t check all platform code.
and this code may result in kernel panic on these platforms.
i think x86’s copy_from{to}_user code need to change like other platforms.
or am i missing something ?
Thanks
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | Borislav Petkov <bp@suse.de> |
|---|---|
| Date | 2015-08-12 12:10 +0200 |
| Message-ID | <pWB8R-1df-5@gated-at.bofh.it> |
| In reply to | #1205724 |
On Wed, Aug 12, 2015 at 05:01:14PM +0800, yalin wang wrote:
> hi x86 maintainers,
>
> i have a question about copy_from{to}_user() function,
> i find on other platforms like arm/ arm64 /hexagon,
> all copy_from{to}_user function only check source address for
> copy_from and only check to address for copy_to user function,
> never check both source and dest together,
>
> but on x86 platform, i see copy_from{to}_user use a generic function
> named copy_user_generic_unrolled() in arch/x86/lib/copy_user_64.S,
That one is the fallback and used only on machines which don't set
X86_FEATURE_REP_GOOD or X86_FEATURE_ERMS. Basically old P4 and K7 and
early K8s.
> it check source and dest address no matter it is copy_from user or
> copy_to_user , is it correct?
> for copy_from_user i think only need check source address is enough,
How else would we be able to use the same function in copy_to and
copy_from variants?
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | yalin wang <yalin.wang2010@gmail.com> |
|---|---|
| Date | 2015-08-13 12:10 +0200 |
| Message-ID | <pWXCq-7m-3@gated-at.bofh.it> |
| In reply to | #1205877 |
> On Aug 12, 2015, at 18:07, Borislav Petkov <bp@suse.de> wrote:
>
> On Wed, Aug 12, 2015 at 05:01:14PM +0800, yalin wang wrote:
>> hi x86 maintainers,
>>
>> i have a question about copy_from{to}_user() function,
>> i find on other platforms like arm/ arm64 /hexagon,
>> all copy_from{to}_user function only check source address for
>> copy_from and only check to address for copy_to user function,
>> never check both source and dest together,
>>
>> but on x86 platform, i see copy_from{to}_user use a generic function
>> named copy_user_generic_unrolled() in arch/x86/lib/copy_user_64.S,
>
> That one is the fallback and used only on machines which don't set
> X86_FEATURE_REP_GOOD or X86_FEATURE_ERMS. Basically old P4 and K7 and
> early K8s.
>
i see, generically, it use 3 function for different processors,
static __always_inline __must_check unsigned long
copy_user_generic(void *to, const void *from, unsigned len)
{
unsigned ret;
/*
* If CPU has ERMS feature, use copy_user_enhanced_fast_string.
* Otherwise, if CPU has rep_good feature, use copy_user_generic_string.
* Otherwise, use copy_user_generic_unrolled.
*/
alternative_call_2(copy_user_generic_unrolled,
copy_user_generic_string,
X86_FEATURE_REP_GOOD,
copy_user_enhanced_fast_string,
X86_FEATURE_ERMS,
ASM_OUTPUT2("=a" (ret), "=D" (to), "=S" (from),
"=d" (len)),
"1" (to), "2" (from), "3" (len)
: "memory", "rcx", "r8", "r9", "r10", "r11");
return ret;
}
>> it check source and dest address no matter it is copy_from user or
>> copy_to_user , is it correct?
>> for copy_from_user i think only need check source address is enough,
>
> How else would we be able to use the same function in copy_to and
> copy_from variants?
for 3 methods implemented here, i think can implemented by add one more function parameter,
like this:
#define COPY_FROM 0
#define COPY_TO 1
#define COPY_IN 2
copy_user_generic(void *to, const void *from, unsigned len, int type)
we store type into one fix register, for example r12 ,
then in fix up code, we can know the exception is caused by copy_from
copy_to or copy_in user function by check r12 value(0 , 1 ,2 value), then if
it is copy_from, we only allow read fault, if the exception is write fault, panic() .
the same rules also apply to copy_to / copy_in function .
is it possible to change it like this ?
Thanks
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Borislav Petkov <bp@suse.de> |
|---|---|
| Date | 2015-08-13 18:50 +0200 |
| Message-ID | <pX3Rw-w6-7@gated-at.bofh.it> |
| In reply to | #1206670 |
On Thu, Aug 13, 2015 at 06:04:54PM +0800, yalin wang wrote:
> we store type into one fix register, for example r12 ,
> then in fix up code, we can know the exception is caused by copy_from
> copy_to or copy_in user function by check r12 value(0 , 1 ,2 value), then if
> it is copy_from, we only allow read fault, if the exception is write fault, panic() .
>
> the same rules also apply to copy_to / copy_in function .
>
> is it possible to change it like this ?
... and we'll do all that jumping through hoops to fix what actual,
real-life problem exactly?
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | yalin wang <yalin.wang2010@gmail.com> |
|---|---|
| Date | 2015-08-17 05:30 +0200 |
| Message-ID | <pYjhw-3w8-7@gated-at.bofh.it> |
| In reply to | #1206959 |
> On Aug 14, 2015, at 00:43, Borislav Petkov <bp@suse.de> wrote:
>
> On Thu, Aug 13, 2015 at 06:04:54PM +0800, yalin wang wrote:
>> we store type into one fix register, for example r12 ,
>> then in fix up code, we can know the exception is caused by copy_from
>> copy_to or copy_in user function by check r12 value(0 , 1 ,2 value), then if
>> it is copy_from, we only allow read fault, if the exception is write fault, panic() .
>>
>> the same rules also apply to copy_to / copy_in function .
>>
>> is it possible to change it like this ?
>
> ... and we'll do all that jumping through hoops to fix what actual,
> real-life problem exactly?
i just want the x86 copy_from{to,in}_user() function have
the same behaviour as other platforms.
and can disclose potential BUGs in kernel, if do like this.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Borislav Petkov <bp@suse.de> |
|---|---|
| Date | 2015-08-17 06:20 +0200 |
| Message-ID | <pYk3U-4GK-3@gated-at.bofh.it> |
| In reply to | #1208336 |
On Mon, Aug 17, 2015 at 11:27:01AM +0800, yalin wang wrote:
> i just want the x86 copy_from{to,in}_user() function have
> the same behaviour as other platforms.
Back to the original question from 2 mails ago:
How else would we be able to use the same function in copy_to and
copy_from variants?
> and can disclose potential BUGs in kernel, if do like this.
Back to my other question:
Do you have any real life examples where you can trigger such bugs or is
this only "potential"?
IOW, what I *think* you're trying to do sounds to me like unnecessary
complication with no apparent gain *at* *all*. So show me why you want
to do it: code it up, trigger a bug and show me why your version is
better. No "but but it might be a good idea", no "potentially maybe",
none of that maybe stuff. Write it, send it with instructions how
someone else can apply it and trigger the issue. Ok?
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web