From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751628AbdEJGeL (ORCPT ); Wed, 10 May 2017 02:34:11 -0400 Received: from smtp-sh2.infomaniak.ch ([128.65.195.6]:47108 "EHLO smtp-sh2.infomaniak.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751235AbdEJGeJ (ORCPT ); Wed, 10 May 2017 02:34:09 -0400 Subject: Re: [PATCH v2] LSM: Enable multiple calls to security_add_hooks() for the same LSM To: Kees Cook References: <20170509230853.8017-1-mic@digikod.net> Cc: LKML , Casey Schaufler , James Morris , "Serge E . Hallyn" , Tetsuo Handa , linux-security-module From: =?UTF-8?Q?Micka=c3=abl_Sala=c3=bcn?= Message-ID: <7268398c-98e4-ffe7-a76a-0d2597b41048@digikod.net> Date: Wed, 10 May 2017 08:33:45 +0200 User-Agent: MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="tM2GQK3OO6JTV0VhTeTSu54OSlxBN3KAq" X-Antivirus: Dr.Web (R) for Unix mail servers drweb plugin ver.6.0.2.8 X-Antivirus-Code: 0x100000 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) --tM2GQK3OO6JTV0VhTeTSu54OSlxBN3KAq Content-Type: multipart/mixed; boundary="3dLOe92bnBU5JfhavRpcKhXOgCDbPJeNH"; protected-headers="v1" From: =?UTF-8?Q?Micka=c3=abl_Sala=c3=bcn?= To: Kees Cook Cc: LKML , Casey Schaufler , James Morris , "Serge E . Hallyn" , Tetsuo Handa , linux-security-module Message-ID: <7268398c-98e4-ffe7-a76a-0d2597b41048@digikod.net> Subject: Re: [PATCH v2] LSM: Enable multiple calls to security_add_hooks() for the same LSM References: <20170509230853.8017-1-mic@digikod.net> In-Reply-To: --3dLOe92bnBU5JfhavRpcKhXOgCDbPJeNH Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 10/05/2017 01:35, Kees Cook wrote: > On Tue, May 9, 2017 at 4:08 PM, Micka=C3=ABl Sala=C3=BCn wrote: >> The commit d69dece5f5b6 ("LSM: Add /sys/kernel/security/lsm") extend >> security_add_hooks() with a new parameter to register the LSM name, >> which may be useful to make the list of currently loaded LSM available= >> to userspace. However, there is no clean way for an LSM to split its >> hook declarations into multiple files, which may reduce the mess with >> all the included files (needed for LSM hook argument types) and make t= he >> source code easier to review and maintain. >> >> This change allows an LSM to register multiple times its hook while >> keeping a consistent list of LSM names as described in >> Documentation/security/LSM.txt . The list reflects the order in which >> checks are made. This patch only check for the last registered LSM. If= >> an LSM register multiple times its hooks, interleaved with other LSM >> registrations (which should not happen), its name will still appear in= >> the same order that the hooks are called, hence multiple times. >> >> To sum up, "capability,selinux,foo,foo" will be replaced with >> "capability,selinux,foo", however "capability,foo,selinux,foo" will >> remain as is. >> >> Signed-off-by: Micka=C3=ABl Sala=C3=BCn >> Cc: Casey Schaufler >> Cc: James Morris >> Cc: Kees Cook >> Cc: Serge E. Hallyn >> Cc: Tetsuo Handa >> Link: https://lkml.kernel.org/r/ccad825b-7a58-e499-e51b-bd7c98581afe@s= chaufler-ca.com >> --- >> security/security.c | 30 ++++++++++++++++++++++++++++++ >> 1 file changed, 30 insertions(+) >> >> diff --git a/security/security.c b/security/security.c >> index 549bddcc2116..6be65050b268 100644 >> --- a/security/security.c >> +++ b/security/security.c >> @@ -25,6 +25,7 @@ >> #include >> #include >> #include >> +#include >> #include >> >> #define MAX_LSM_EVM_XATTR 2 >> @@ -86,6 +87,32 @@ static int __init choose_lsm(char *str) >> } >> __setup("security=3D", choose_lsm); >> >> +static bool match_last_lsm(const char *list, const char *last) >> +{ >> + size_t list_len, last_len, i; >> + >> + if (!list || !last) >> + return false; >> + list_len =3D strlen(list); >> + last_len =3D strlen(last); >> + if (!last_len || !list_len) >> + return false; >> + if (last_len > list_len) >> + return false; >> + >> + for (i =3D 0; i < last_len; i++) { >> + if (list[list_len - 1 - i] !=3D last[last_len - 1 - i]= ) >> + return false; >> + } >> + /* Check if last_len =3D=3D list_len */ >> + if (i =3D=3D list_len) >> + return true; >> + /* Check if it is a full name */ >> + if (list[list_len - 1 - i] =3D=3D ',') >> + return true; >> + return false; >> +} >=20 > This could be reduced to just: >=20 > static bool match_last_lsm(const char *list, const char *lsm) > { > char *last; >=20 > last =3D strrchr(list, ',') ?: list; >=20 > return (strcmp(last, lsm) =3D=3D 0); > } Indeed, almost the same, I just need to increment and check "last" to make it work. :) I'll keep the initial NULL checks, wrap them with a WARN_ON() and use a const "last", though. It will be much more readable, thanks! >=20 >> + >> static int lsm_append(char *new, char **result) >> { >> char *cp; >> @@ -93,6 +120,9 @@ static int lsm_append(char *new, char **result) >> if (*result =3D=3D NULL) { >> *result =3D kstrdup(new, GFP_KERNEL); >> } else { >> + /* Check if it is the last registered name */ >> + if (match_last_lsm(*result, new)) >> + return 0; >> cp =3D kasprintf(GFP_KERNEL, "%s,%s", *result, new); >> if (cp =3D=3D NULL) >> return -ENOMEM; >> -- >> 2.11.0 >> >=20 >=20 >=20 --3dLOe92bnBU5JfhavRpcKhXOgCDbPJeNH-- --tM2GQK3OO6JTV0VhTeTSu54OSlxBN3KAq Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAEBCgAdFiEEUysCyY8er9Axt7hqIt7+33O9apUFAlkStEkACgkQIt7+33O9 apXv8AgAm3CmFqwO0eM+2j6/200BMq+aMGc/95Ts6ao1PxoT4kTz75Ci8fgwdZFA qk6Kgh2s1NThX3FVED93HI9E8JSpcW8ouwi2vclM+oQror6kheX1X+wen+g1yA/E YKIV7QASeyHMZnQc164jeJIZyOAenBYiYW7aA6wps+Vf8PjKIYJ7xtUVG1WX/CGv AIiQ3095K0TxtjWObfdRKeDALeuPMUFN5OaBNgzqH9tOnqSPIH0+88avIHRp0tbx saZrQek19IiQS+i/6kvAL41ICmsmMU5bX8F3kUwU0iQIGiDSnJw482mnzlKSaraN Be5UP7Th8XGKpZHUIL4fwKFNYglj7w== =NsUU -----END PGP SIGNATURE----- --tM2GQK3OO6JTV0VhTeTSu54OSlxBN3KAq--