From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755590AbcGEVew (ORCPT ); Tue, 5 Jul 2016 17:34:52 -0400 Received: from nm41-vm4.bullet.mail.bf1.yahoo.com ([216.109.114.159]:55304 "EHLO nm41-vm4.bullet.mail.bf1.yahoo.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752488AbcGEVet (ORCPT ); Tue, 5 Jul 2016 17:34:49 -0400 X-Yahoo-Newman-Id: 78643.4724.bm@smtp204.mail.bf1.yahoo.com X-Yahoo-Newman-Property: ymail-3 X-YMail-OSG: 6Tqi.oUVM1nl0HrUOGRquF_F3Ztbsz.TZhWYhyV7bgrhPpZ VzUfa4ygen_wQmoSei09rDRNua58glr_ZFChYhbCHVkfFXdXtTkq9f.icPjw FkuhW5zXkh6fzFVI.hlVPLrhoV3i82PIamLVyGJoqQ.Isp8rzMLVUHzwTT0L V4EL266zxgZ8ByM0DD2D4CI4nOalg65mrb6KREHwfjxZVthWrbcLnARyaGSH F1uImNN8DkWFdoUwllncj9SSajzhhPEWnlhg9Fbt2j6BiryA7x7Y5XZj1qm6 t3y0Ou5llqIqVQSrjuH4JEaYHcxukx2BxPQhaqX0z2l29g.wRvMyD1ili9cA HLx4J66YhamZWRL2AeEnynsEgg5IMJXCu5VIrFJ1oiOvXvIXUyu6GpTMKhI_ 8J4pA5PKKW_uELiIA63jL01bfoVMMEl5rZu_g5bZ7LYt4SzCZnXM6ULRI8T0 1_GmDXZou5kBYMX6g7f3jFHWhWAh3F5fivL2P0kst_Irt7YPJGFVzHm1n3AS 1ARGA4vX3Cw1gSqpRB5FxmA.bCZjC9S0Rz6moZPRVNWOI7sKR X-Yahoo-SMTP: OIJXglSswBDfgLtXluJ6wiAYv6_cnw-- Subject: Re: [PATCH 2/5] security,overlayfs: Provide security hook for copy up of xattrs for overlay file To: Vivek Goyal References: <1467733854-6314-1-git-send-email-vgoyal@redhat.com> <1467733854-6314-3-git-send-email-vgoyal@redhat.com> <20160705211502.GG17987@redhat.com> Cc: miklos@szeredi.hu, sds@tycho.nsa.gov, linux-kernel@vger.kernel.org, linux-unionfs@vger.kernel.org, linux-security-module@vger.kernel.org, dwalsh@redhat.com, dhowells@redhat.com, pmoore@redhat.com, viro@ZenIV.linux.org.uk, linux-fsdevel@vger.kernel.org From: Casey Schaufler Message-ID: <93ab3b87-a9c7-b792-2253-07aa2d03681e@schaufler-ca.com> Date: Tue, 5 Jul 2016 14:34:43 -0700 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 MIME-Version: 1.0 In-Reply-To: <20160705211502.GG17987@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 7/5/2016 2:15 PM, Vivek Goyal wrote: > On Tue, Jul 05, 2016 at 01:22:22PM -0700, Casey Schaufler wrote: >> On 7/5/2016 8:50 AM, Vivek Goyal wrote: >>> Provide a security hook which is called when xattrs of a file are being >>> copied up. This hook is called once for each xattr and one can either >>> accept or reject xattr. If 0 is returned, xattr will be copied up, if 1 >>> is returned, xattr will not be copied up and if negative error code >>> is returned, copy up will be aborted. >>> >>> In SELinux, label of lower file is not copied up. File already has been >>> set with right label at the time of creation and we don't want to overwrite >>> that label. >>> >>> Signed-off-by: David Howells >>> Signed-off-by: Vivek Goyal >>> --- >>> fs/overlayfs/copy_up.c | 8 ++++++++ >>> include/linux/lsm_hooks.h | 13 +++++++++++++ >>> include/linux/security.h | 10 ++++++++++ >>> security/security.c | 9 +++++++++ >>> security/selinux/hooks.c | 14 ++++++++++++++ >>> 5 files changed, 54 insertions(+) >>> >>> diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c >>> index 90dc362..2c31938 100644 >>> --- a/fs/overlayfs/copy_up.c >>> +++ b/fs/overlayfs/copy_up.c >>> @@ -103,6 +103,14 @@ retry: >>> goto retry; >>> } >>> >>> + error = security_inode_copy_up_xattr(old, new, >>> + name, value, size); >>> + if (error < 0) >>> + break; >>> + if (error == 1) { >>> + error = 0; >>> + continue; /* Discard */ >>> + } >>> error = vfs_setxattr(new, name, value, size, 0); >>> if (error) >>> break; >>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h >>> index fcde9b9..2a8ee8c 100644 >>> --- a/include/linux/lsm_hooks.h >>> +++ b/include/linux/lsm_hooks.h >>> @@ -412,6 +412,16 @@ >>> * @src indicates the union dentry of file that is being copied up. >>> * @old indicates the pointer to old_cred returned to caller. >>> * Returns 0 on success or a negative error code on error. >>> + * @inode_copy_up_xattr: >>> + * Filter the xattrs being copied up when a unioned file is copied >>> + * up from a lower layer to the union/overlay layer. >>> + * @src indicates the file that is being copied up. >>> + * @dst indicates the file that has being created by the copy up. >>> + * @name indicates the name of the xattr. >>> + * @value, @size indicate the payload of the xattr. >>> + * Returns 0 to accept the xattr, 1 to discard the xattr or a negative >>> + * error code to abort the copy up. Note that the caller is responsible >>> + * for reading and writing the xattrs as this hook is merely a filter. >> The return should be -EOPNOTSUPP from security modules that don't >> support the attribute "name". This will make it possible to support >> multiple modules that provide attributes. (patches pending) > Hmm.., Sorry I did not understand this one. > > So all modules will not understand all xattrs. So if they start returning > -EOPNOTSUPP, then as per current implementation, copy up operation will > be aborted. Yes, the infrastructure code will have to change to deal with the tri-state returns. That's also true of several other hooks. > Current implementation relies on that a security module, returns 0 if > every thing is "name" xattr should be copied up or lsm does not care. > Negative error code is returned only if something is wrong. Given every > lsm will not understand/care about all the xattrs, we can't return > error code if lsm does not own/understand the "name". In fact > call_int_hook() will bail out the very first time negative error code > is returned. > > IOW, current implementation will work with multiple modules providing > implementation for same hook as long as module returns 0 for the xattrs > it does not understand. There have to be four states. I own this attribute, and want you to use it. I own this attribute and I want you to ignore it. I don't own this attribute. I own this attribute and something went terribly wrong, such as running out of memory. > > I guess I am missing something. Can you please elaborate a little more. > >> If the only use to which this hook is put is to identify attributes >> that should be discarded it's unnecessary overhead to pass the >> parameters that are never used. > Ok, I will get rid of extra parameters. If somebody needs these, it can > be added later. > > Vivek >