From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754294AbbHMWt5 (ORCPT ); Thu, 13 Aug 2015 18:49:57 -0400 Received: from mail-ig0-f181.google.com ([209.85.213.181]:34754 "EHLO mail-ig0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752698AbbHMWtz (ORCPT ); Thu, 13 Aug 2015 18:49:55 -0400 MIME-Version: 1.0 In-Reply-To: References: <55CCB510.3060807@redhat.com> <55CD0DAC.9080809@redhat.com> Date: Thu, 13 Aug 2015 15:49:54 -0700 X-Google-Sender-Auth: MZibnvDHqIcBFF7VAtyxQBB6qyg Message-ID: Subject: Re: [Regression v4.2 ?] 32-bit seccomp-BPF returned errno values wrong in VM? From: Linus Torvalds To: Andy Lutomirski Cc: Denys Vlasenko , Kees Cook , David Drysdale , "linux-kernel@vger.kernel.org" , Will Drewry , Ingo Molnar , Alok Kataria , Borislav Petkov , Alexei Starovoitov , Frederic Weisbecker , "H. Peter Anvin" , Oleg Nesterov , Steven Rostedt , X86 ML Content-Type: multipart/mixed; boundary=089e013a114e66f518051d3927ca Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --089e013a114e66f518051d3927ca Content-Type: text/plain; charset=UTF-8 On Thu, Aug 13, 2015 at 2:47 PM, Andy Lutomirski wrote: > > It seems to me that the bug is that sysexit_from_sys_call isn't > reloading RAX from regs->ax. Ugh. That code is confusing, and _most_ cases seem to have %rax already loaded. There seems to be three cases: - fallthrough from cstar_dispatch after a successful call to the system call This has %rax as the correct return value (which also got saved to RAX on stack) - the 'auditsys_exit' macro 'exit' case. This seems to have %rax reloaded inside the macro - the out-of-range system call case for cstar_dispatch This does *not* seem to load %rax with $ENOSYS, but keeps the bad system call number in it. so yeah, there seems to be a bug there, but if I read it right, that bug seems to happen just for the out-of-range system call case, which afaik isn't the case reported here. I guess adding a re-load of %rax is ok, even though in the common cases it is already loaded. Oh, and sysexit_from_sys_call seems to have the exact same situation. The "system call dispatch with %rax out of range" fallthrough case doesn't set %rax to ENOSYS. So I guess we could remove the reloading of system call return value from auditsys_exit, and just do it unconditionally in the common path. Which is sad, since the *really* common case already has the right value, but whatever. Does the attached patch make sense and work? Totally untested, just looking at the code. But maybe it's right, because it's exactly that ENOSYS case that the bad patch in question changed. Btw, the old ENOSYS code also cleared ORIG_EAX. I'm not sure why, but we used to have ia32_badsys: movq $0,ORIG_RAX(%rsp) movq $-ENOSYS,%rax jmp ia32_sysret for that case. Linus --089e013a114e66f518051d3927ca Content-Type: text/plain; charset=US-ASCII; name="patch.diff" Content-Disposition: attachment; filename="patch.diff" Content-Transfer-Encoding: base64 X-Attachment-Id: f_idastiai0 IGFyY2gveDg2L2VudHJ5L2VudHJ5XzY0X2NvbXBhdC5TIHwgMyArKy0KIDEgZmlsZSBjaGFuZ2Vk LCAyIGluc2VydGlvbnMoKyksIDEgZGVsZXRpb24oLSkKCmRpZmYgLS1naXQgYS9hcmNoL3g4Ni9l bnRyeS9lbnRyeV82NF9jb21wYXQuUyBiL2FyY2gveDg2L2VudHJ5L2VudHJ5XzY0X2NvbXBhdC5T CmluZGV4IDVhMTg0NDc2NWE3YS4uYTdlMjU3ZDljYjkwIDEwMDY0NAotLS0gYS9hcmNoL3g4Ni9l bnRyeS9lbnRyeV82NF9jb21wYXQuUworKysgYi9hcmNoL3g4Ni9lbnRyeS9lbnRyeV82NF9jb21w YXQuUwpAQCAtMTQwLDYgKzE0MCw3IEBAIHN5c2V4aXRfZnJvbV9zeXNfY2FsbDoKIAkgKi8KIAlh bmRsCSR+VFNfQ09NUEFULCBBU01fVEhSRUFEX0lORk8oVElfc3RhdHVzLCAlcnNwLCBTSVpFT0Zf UFRSRUdTKQogCW1vdmwJUklQKCVyc3ApLCAlZWN4CQkvKiBVc2VyICVlaXAgKi8KKwltb3ZxICAg IFJBWCglcnNwKSwgJXJheAogCVJFU1RPUkVfUlNJX1JESQogCXhvcmwJJWVkeCwgJWVkeAkJLyog RG8gbm90IGxlYWsga2VybmVsIGluZm9ybWF0aW9uICovCiAJeG9ycQklcjgsICVyOApAQCAtMjE5 LDcgKzIyMCw2IEBAIHN5c2V4aXRfZnJvbV9zeXNfY2FsbDoKIDE6CXNldGJlCSVhbAkJCS8qIDEg aWYgZXJyb3IsIDAgaWYgbm90ICovCiAJbW92emJsCSVhbCwgJWVkaQkJLyogemVyby1leHRlbmQg dGhhdCBpbnRvICVlZGkgKi8KIAljYWxsCV9fYXVkaXRfc3lzY2FsbF9leGl0Ci0JbW92cQlSQVgo JXJzcCksICVyYXgJCS8qIHJlbG9hZCBzeXNjYWxsIHJldHVybiB2YWx1ZSAqLwogCW1vdmwJJChf VElGX0FMTFdPUktfTUFTSyAmIH5fVElGX1NZU0NBTExfQVVESVQpLCAlZWRpCiAJRElTQUJMRV9J TlRFUlJVUFRTKENMQlJfTk9ORSkKIAlUUkFDRV9JUlFTX09GRgpAQCAtMzY4LDYgKzM2OCw3IEBA IHN5c3JldGxfZnJvbV9zeXNfY2FsbDoKIAlSRVNUT1JFX1JTSV9SRElfUkRYCiAJbW92bAlSSVAo JXJzcCksICVlY3gKIAltb3ZsCUVGTEFHUyglcnNwKSwgJXIxMWQKKwltb3ZxICAgIFJBWCglcnNw KSwgJXJheAogCXhvcnEJJXIxMCwgJXIxMAogCXhvcnEJJXI5LCAlcjkKIAl4b3JxCSVyOCwgJXI4 Cg== --089e013a114e66f518051d3927ca--