Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   "Li, Xin3" <xin3.li@intel.com>
To:     "Eric W. Biederman" <ebiederm@xmission.com>
CC:     "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "x86@kernel.org" <x86@kernel.org>,
        "tglx@linutronix.de" <tglx@linutronix.de>,
        "mingo@redhat.com" <mingo@redhat.com>,
        "bp@alien8.de" <bp@alien8.de>,
        "dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
        "hpa@zytor.com" <hpa@zytor.com>,
        "brgerst@gmail.com" <brgerst@gmail.com>
Subject: RE: [PATCH] x86/ia32: Do not modify the DPL bits for a null selector
Thread-Topic: [PATCH] x86/ia32: Do not modify the DPL bits for a null selector
Thread-Index: AQHZr83AFvT4TTcrIkq3d3Abr5XF0q+sxk7ngAC/myA=
Date:   Fri, 7 Jul 2023 04:07:30 +0000
Message-ID: <SA1PR11MB67348D11385F584354AB84C3A82DA@SA1PR11MB6734.namprd11.prod.outlook.com>
References: <20230706052231.2183-1-xin3.li@intel.com>
 <87v8exgmot.fsf@email.froward.int.ebiederm.org>
In-Reply-To: <87v8exgmot.fsf@email.froward.int.ebiederm.org>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?kisK+eh5v+Sxtv0ugFa4reNMo5rVIvE2c6qlOW20f3ePDuE1JnWMVOaGj4V0?=
 =?us-ascii?Q?09j6+/H/UPQxXfbifzMqT5lqf/1WqUDRIGwUb/HukoEXGGghODl5AX++GB8T?=
 =?us-ascii?Q?lu4r05LDe+dkpekTjzgFdaXkHMEbb78N1XGeE3MnAyk5lDcWRf8/KHQAmYyE?=
 =?us-ascii?Q?VVTa5hLMYv1V02bPkNt4mnqQAvUrsf9Kb9j1SaKQTbvsqVkQPs4gDtweKVW3?=
 =?us-ascii?Q?WpA8kriJXDWRP9zUG+2eksOIsE7CdQ8n8uYt168MgfqshhdHT41ltoc9rmME?=
 =?us-ascii?Q?MLB6i4+B/DsfJVGofyVzeANYQZ5puFWucFZtt6r6MGCf4uYsB4bfKPdQYHn3?=
 =?us-ascii?Q?yfF2+8yVNceHkqA56By+64ho+6R1Gn9Wud/mMx2kJVKojLixMjQTHNFhEPNZ?=
 =?us-ascii?Q?OOMqoS0GP6kwEuxlKICoFkuCfu7WlJNdPwpKuAXFktm17c1PHNBBP72fpyiG?=
 =?us-ascii?Q?VKwYnmYSXMyQBvoFHLmeMPSE7Jbq4Xm4U2kQ/GND8O53L+7Q0DcMLOFXJy0g?=
 =?us-ascii?Q?Su0TrQO5BTexw5f9WXSJDupBKIg40jbbnmOMpsaMO+1/5YukiakaNeYkkGwt?=
 =?us-ascii?Q?oKnLdot5W02zumqUU4PzLg5YtpoqgWCx4AxRUiATC3VsKOYku5yMg2IqpDDd?=
 =?us-ascii?Q?sUDQ97qg727P73OwfiqH1O051J7EMbs28yfqTCfVKb3KI4VuUXZfdxHQ7x4M?=
 =?us-ascii?Q?hjpC25bsftpvYZlnamOTbcHFY1Y2XGOeGYQ+gsj6MhPRUpT+vrPgALD3ptFW?=
 =?us-ascii?Q?wt7uNuT2A6eSQSCJqm24QdBabHTQzImoK473EMHhY3TOjRlUMjF3gmTpJadW?=
 =?us-ascii?Q?WcSE0QLrAzCHMUChM4hOJdhKLUnQAI6eGKsYi5+uKqx43ZgOIbB0Iv5h67ec?=
 =?us-ascii?Q?F2fPMoheo/iHRG8LUj3k4JFeOs89gZqpH7t584243si92blh3hrWQ8soqj/Y?=
 =?us-ascii?Q?6kjV16uH2gbAM07gsSrjV+T4xSNZs9CLe5PXsEBT1hRvM+CXOAhhh3ybvNgi?=
 =?us-ascii?Q?ShF+NweM++EXj0ZZx2fELZykKhRDI6NR8xdvDLPp2yoQR72AlER1g3ZGW1W+?=
 =?us-ascii?Q?M0P2RUBtarOEKtqiQghNnSnni0NKRCJLRHYvhT0LqLieoagfYRYySPx+cR03?=
 =?us-ascii?Q?N01ZYPBUXlAopGcdtYQ/yCH8Bn01LjYejQY/8eFlUAvXo3iWcMsPY2ZOTKwM?=
 =?us-ascii?Q?8NEJ8oo5gkQPMdA+3aOMmV8OSK4Jce4i8uYBFVLvn6LXY/JgPTqIdF+R3ZJc?=
 =?us-ascii?Q?//fIVkU/ery81ghEbSnkYoCr7zp98hB7AltFhOcV+Fv+OqyAUmKwCMPAOU6f?=
 =?us-ascii?Q?IBGUq8nNLoS/1jqlOt//v4WO3ByjmCtGJetp+T4RVa7mSuKu/WhVjLxVF22C?=
 =?us-ascii?Q?IXVxGD1mfKsIQ26fXd+ZOyEENTMmRmbKIqP/y+pK48kqUIhTyBVGCn+12rxe?=
 =?us-ascii?Q?VTMadzGcZBi4zPmaMQVARpaPK4rbnoOKrJnaHVD2NxsMRMUcdRt3hM2IniSU?=
 =?us-ascii?Q?BfzbgKiv7274ClvfCKArirY8MSLkZEjhNhgM6egLrF3L8FbJdhjhinKczMkZ?=
 =?us-ascii?Q?r1iM9oJCrcSn8se6f4k=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: SA1PR11MB6734.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: e9fc3009-2acd-4cf2-8c99-08db7e9faed1
X-MS-Exchange-CrossTenant-originalarrivaltime: 07 Jul 2023 04:07:30.0679
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: F8i4xZ8UjECx8WaBhuuEi9xcZ+04AgEOxjx+dXLSYfAyAUsSzbkzoEYdzitlQqz3OynYQGg0ZMRdlSPo7sjd7Q==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR11MB6122
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> > When a null selector is to be loaded into a segment register,
> > reload_segments() sets its DPL bits to 3. Later when an IRET
> > instruction loads it, it zeros the segment register. The two
> > operations offset each other to actually effect a nop.
> >
> > Fix it by not modifying the DPL bits for a null selector.
>=20
> Maybe this is the right thing but this needs some serious comments about =
what is
> going on.
>=20
> In particular how does sel <=3D 3 equate to a null selector?  Is that def=
ined
> somewhere?

In protected mode, a NULL selector (values 0000 through 0003) can be
loaded into DS, ES, FS, or GS registers without causing a protection
exception.

This can be found at the description of instruction LDS/LES/LFS/LGS/LSS,
in section 3.3 "instructions (A-L)" of Intel SDM Volume 2, Instruction Set
Reference.
=20
>At a minimum you should have static asserts to make certain no
> one redefines the first 4 segment selectors as anything else, if you want=
 to refer to
> them by number instead of testing for specific properties.

Bits 0 and 1 of a selector are NOT used to index the GDT or IDT, thus
selector values 0 ~ 3 all point to the first entry of the GDT, i.e.,
the null selector.

Thus 3 as a selector is the same as 0, and it doesn't matter to change
it or not. But when IRET sees an invalid segment register in ES, FS, GS,
and DS, it sets it to 0, making 0 a preferred null selector value.

The sigreturn selftest sets DS of its signal context to 0 and then checks
if it's still 0 after sigreturn. And it passes. However if it sets DS to
any other null selector value, e.g., 1, it fails. For FRED, if we don't
modify the DPL bits, the test passes, because ERETU doesn't change segment
registers.

> As written this looks like it requires an enormous amount of knowledge ab=
out
> how other parts of the code works, to be comprehensible or to change safe=
ly.
> That level of non-local knowledge should be unnecessary.