From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751709AbdBNAZX (ORCPT ); Mon, 13 Feb 2017 19:25:23 -0500 Received: from youngberry.canonical.com ([91.189.89.112]:40946 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751670AbdBNAZV (ORCPT ); Mon, 13 Feb 2017 19:25:21 -0500 Subject: Re: [PATCH v2 1/4] seccomp: Add sysctl to display available actions To: Kees Cook References: <1486100262-32391-1-git-send-email-tyhicks@canonical.com> <1486100262-32391-2-git-send-email-tyhicks@canonical.com> Cc: Paul Moore , Eric Paris , Andy Lutomirski , Will Drewry , linux-audit@redhat.com, LKML From: Tyler Hicks Message-ID: Date: Mon, 13 Feb 2017 18:25:10 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="0OH5Hn1VlF8rjoABJdOjeI595s71cVk7E" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --0OH5Hn1VlF8rjoABJdOjeI595s71cVk7E Content-Type: multipart/mixed; boundary="qlboaVEtgeihaKUqLxG8Fwrxj5LawJ18f"; protected-headers="v1" From: Tyler Hicks To: Kees Cook Cc: Paul Moore , Eric Paris , Andy Lutomirski , Will Drewry , linux-audit@redhat.com, LKML Message-ID: Subject: Re: [PATCH v2 1/4] seccomp: Add sysctl to display available actions References: <1486100262-32391-1-git-send-email-tyhicks@canonical.com> <1486100262-32391-2-git-send-email-tyhicks@canonical.com> In-Reply-To: --qlboaVEtgeihaKUqLxG8Fwrxj5LawJ18f Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 02/07/2017 06:43 PM, Kees Cook wrote: > On Tue, Feb 7, 2017 at 4:25 PM, Tyler Hicks wro= te: >> On 02/07/2017 06:03 PM, Kees Cook wrote: >>> On Thu, Feb 2, 2017 at 9:37 PM, Tyler Hicks w= rote: >>>> This patch creates a read-only sysctl containing an ordered list of >>>> seccomp actions that the kernel supports. The ordering, from left to= >>>> right, is the lowest action value (kill) to the highest action value= >>>> (allow). Currently, a read of the sysctl file would return "kill tra= p >>>> errno trace allow". The contents of this sysctl file can be useful f= or >>>> userspace code as well as the system administrator. >>>> >>>> The path to the sysctl is: >>>> >>>> /proc/sys/kernel/seccomp/actions_avail >>>> >>>> libseccomp and other userspace code can easily determine which actio= ns >>>> the current kernel supports. The set of actions supported by the cur= rent >>>> kernel may be different than the set of action macros found in kerne= l >>>> headers that were installed where the userspace code was built. >>> >>> This is certainly good: having a discoverable way to detect filter >>> capabilities. I do wonder if it'd still be easier to just expose the >>> max_log sysctl as a numeric value, since the SECCOMP_RET_* values are= >>> all part of uapi, so we can't escape their values... >> >> I was very torn on whether to use a numeric or string representation >> here. The reason I decided on string representation is because I think= >> these sysctls are mostly aimed for admins and numeric representations >> wouldn't be easy to use. I considered added a utility to libseccomp bu= t, >> since the kernel code to do a string representation was so simple, I >> went with doing it in the kernel. >=20 > Yeah, I think I like it just because it gives a way to discover the > UAPI "level"... I will think more about this. For v3, let's keep the > string stuff. >=20 >> Another possibility is exposing the SECCOMP_RET_*_NAME macros as part = of >> the uapi. >=20 > I like keeping the UAPI minimal. ;) >=20 >>>> +static int __init seccomp_sysctl_init(void) >>>> +{ >>>> + struct ctl_table_header *hdr; >>>> + >>>> + hdr =3D register_sysctl_paths(seccomp_sysctl_path, seccomp_s= ysctl_table); >>>> + kmemleak_not_leak(hdr); >>> >>> Will kmemleak complain about this if hdr is saved to a global (or not= >>> saved at all)? Also, something should be reported in the failure >>> case... >> >> I have to admit to blindly following the example set by sysctl_init() = in >> kernel/sysctl.c. I can test what kmemleak will/won't complain about an= d >> report back (tomorrow at the earliest). >=20 > Cool, no rush. I'm backlogged on reviews anyway. :) kmemleak doesn't complain if we save it to a global. That makes sense because it means that we have a persistent reference to the allocated memory. However, kmemleak doesn't complain about this allocation as-is (meaning that I simply removed the call to kmemleak_not_leak()). From what I can tell, this is because a reference to the allocated ctl_table_header struct is saved when __register_sysctl_table() calls init_header(). I think kmemleak is seeing this reference when doing scans and (incorrectly) thinking that there's no leak. I think the safest/cleanest thing to do is leave the call to kmemleak_not_leak(). Let me know if you disagree. Tyler --qlboaVEtgeihaKUqLxG8Fwrxj5LawJ18f-- --0OH5Hn1VlF8rjoABJdOjeI595s71cVk7E Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- iQIcBAEBCgAGBQJYok5mAAoJENaSAD2qAscKSCEP/ipV+w5pZCE04khUaopdTDgT gkwpft2E8PlNF6rJxuZqbZquImwGvT5MZaK8tzot2TgEW/FJgJl3jgX5Gq2KqxXe 7T/jfn4GQlyLBOe3MhVk8AfxNkFNog+jkGkjhxkgDLRMQMxMSh8/8jz36+F7FMt0 XHR6jFcFspoawHWM3oFIWpRTHtO8h0+dCz7BdvBz4Wqkif6BRd+41Q0cgvK0j3u0 LbNZl3eAlyqnwADT6+EfFfzpymXxdkZxOZ/pvnoFheyDg/8+QF2NhlfW1KwgkbQB ZbrWmhrUuI+9USlplRYGNeyKCgvGAenIk9rRaCJ6Cy1v+tE8p3tLWjXgaidmVipA 7i3KVrzm/DBExPSGtjqSrstiXUyhJs8C5Q4sBaKdbsHMl8DBU3KYlsTPdBNz39AM TAThhar9TokEppudHP8MTKKE9PTivpsJiLuzNwc+dd+qJzXs/15me9BCS8SaqpmF AaQ6UCTRZ5nVV//PnnbLMbJtNaTRFolYSRB1lCAoHdpgs8KRcOVVDkjRrSAH0umE IY24h3wXQjsn7B9PEF04G/zKkf5HxNaHOoa6dRWBEuhv9nAxNwiWZLUY3ewbDg3p 9LL8Ma5bCroJujG+me13LKYzULPu2rPniEABqdZBRbBMMAzJ/kCjtf42F3hzaDb8 ox5+wh2RM3NXA+Mu34Qu =FAHV -----END PGP SIGNATURE----- --0OH5Hn1VlF8rjoABJdOjeI595s71cVk7E--