2021-10-25 21:13:49

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH v2 10/32] signal/vm86_32: Properly send SIGSEGV when the vm86 state cannot be saved.

On Mon, Oct 25, 2021 at 1:54 PM Eric W. Biederman <[email protected]> wrote:
>
> Update save_v86_state to always complete all of it's work except
> possibly some of the copies to userspace even if save_v86_state takes
> a fault. This ensures that the kernel is always in a sane state, even
> if userspace has done something silly.

Well, honestly, with this change, you might as well replace the
force_sigsegv() with just a plain "force_sig()", and make it something
the process can catch.

The only thing that "force_sigsgv()" does is to make SIGSEGV
uncatchable. In contrast, a plain "force_sig()" just means that it
can't be ignored - but it can be caught, and it is fatal only when not
caught.

And with the "always complete the non-vm86 state restore" part change,
there's really no reason for it to not be caught.

Of course, the other case (where we have no state information for the
"enter vm86 mode" case) is still fatal, and is a "this should never
happen". But the "cannot write to the vm86 save state" thing isn't
technically fatal.

It should even be possible to write a test for it: passing a read-only
pointer to the vm86() system call. The vm86 entry will work (because
it only reads the vm86 state from it), but then at vm86 exit, writing
the state back will fail.

Anybody?

Linus


2021-10-26 04:46:04

by Eric W. Biederman

[permalink] [raw]
Subject: Re: [PATCH v2 10/32] signal/vm86_32: Properly send SIGSEGV when the vm86 state cannot be saved.

Linus Torvalds <[email protected]> writes:

> On Mon, Oct 25, 2021 at 1:54 PM Eric W. Biederman <[email protected]> wrote:
>>
>> Update save_v86_state to always complete all of it's work except
>> possibly some of the copies to userspace even if save_v86_state takes
>> a fault. This ensures that the kernel is always in a sane state, even
>> if userspace has done something silly.
>
> Well, honestly, with this change, you might as well replace the
> force_sigsegv() with just a plain "force_sig()", and make it something
> the process can catch.

The trouble is I don't think there is enough information made available
for user space to do anything with the SIGSEGV. My memory is that
applications like dosemu very much have a SIGSEGV handler.

So I think if it ever happened it could be quite confusing. Not to
mention the pr_alert message.

But I guess if a test is written like you suggest we can include enough
information for someone to make sense of things.

> The only thing that "force_sigsgv()" does is to make SIGSEGV
> uncatchable. In contrast, a plain "force_sig()" just means that it
> can't be ignored - but it can be caught, and it is fatal only when not
> caught.
>
> And with the "always complete the non-vm86 state restore" part change,
> there's really no reason for it to not be caught.
>
> Of course, the other case (where we have no state information for the
> "enter vm86 mode" case) is still fatal, and is a "this should never
> happen". But the "cannot write to the vm86 save state" thing isn't
> technically fatal.
>
> It should even be possible to write a test for it: passing a read-only
> pointer to the vm86() system call. The vm86 entry will work (because
> it only reads the vm86 state from it), but then at vm86 exit, writing
> the state back will fail.
>
> Anybody?

I am enthusiastic about writing a test, but I will plod in that
direction just so I can get this sorted out.

Eric