Re: [PATCH v9 15/29] x86/insn-eval: Add utility functions to get segment descriptor base address and limit

[Borislav Petkov]

On Wed, Oct 11, 2017 at 12:57:01PM -0700, Ricardo Neri wrote:
> This is meant to be an error case. In long mode, onlyÂINAT_SEG_REG_IGNORE/FS/GS
> are valid. All other indices are invalid.
> 
> Perhaps we could return -EINVAL instead?

So, my question is, when are you ever going to have that case? What
constellation of events would ever hit this else branch for long mode?
Because it looks impossible to me. What I can imagine only is something
like this:

                else if (seg_reg != INAT_SEG_REG_IGNORE)
			WARN_ONCE(1, "This should never happen!\n");

assertion.

But you don't really need that - you can simply ignore seg_reg in that
case:

        if (user_64bit_mode(regs)) {
                /*
                 * Only FS or GS will have a base address, the rest of
                 * the segments' bases are forced to 0.
                 */
                unsigned long base;

                if (seg_reg == INAT_SEG_REG_FS)
                        rdmsrl(MSR_FS_BASE, base);
                else if (seg_reg == INAT_SEG_REG_GS)
                        /*
                         * swapgs was called at the kernel entry point. Thus,
                         * MSR_KERNEL_GS_BASE will have the user-space GS base.
                         */
                        rdmsrl(MSR_KERNEL_GS_BASE, base);
                else
                        base = 0;

                return base;
        }

Or am I missing something?

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix ImendÃrffer, Jane Smithard, Graham Norton, HRB 21284 (AG NÃrnberg)
--