From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932182AbbFBS77 (ORCPT ); Tue, 2 Jun 2015 14:59:59 -0400 Received: from smtp106.biz.mail.bf1.yahoo.com ([98.139.244.54]:32338 "EHLO smtp106.biz.mail.bf1.yahoo.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754334AbbFBS7p (ORCPT ); Tue, 2 Jun 2015 14:59:45 -0400 X-Yahoo-Newman-Property: ymail-3 X-YMail-OSG: 9bGRO6AVM1ku1GXm0gXbNkq6gD4uO9PodcmeZM.eqyYja45 TNX7PwZFqevXFvRIUaNo4CsnbRi2YSZrXexS81bJNfyKnt2eJvgNHhSSyCgL B92HOkOQXTVsVODde4r7nCT9E6vm2r7NaMpL2qyQHB4IETrtf0y3qrmammQH h9UnoieURS5uLWzKl88A88uAW7H15y2WguSoBybRJZSqxxzEM9QLrwXG8QDG 1wuw3Cba.fQ_tt4ibwJM8oZUZMeeg79NEt4kbxbeORiHAJaDPky2ndEpq2.a Yu6v1Etd3wVg47eSo8a54_mx2Mvph19RWWLwu_.Q4c9ya4Aq5RuvNWvCl2Hr YRyIAKp2CcJ6KqCRcyWAq15sJbas783LPOKC1nNEMLjPviiGeooGsp1udUI. DE.t8Mhcouur8ngA6WGhmOBUE960kgc9j_cBocNIgvNilO3hDuu1ZJtt.s_C LNRqAhNTnzHsXR.QJWLvVgoSxAg6_dlBAl6_aLCS95o3BR6AMD.t.10Xng7X 90jMh3yMfftGwxRI2KRRxCCS5XbvY7kkMzg-- X-Yahoo-SMTP: OIJXglSswBDfgLtXluJ6wiAYv6_cnw-- Message-ID: <556DFD1E.4060207@schaufler-ca.com> Date: Tue, 02 Jun 2015 11:59:42 -0700 From: Casey Schaufler User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Rafal Krypa CC: James Morris , "Serge E. Hallyn" , linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/2] Smack: fix seq operations in smackfs References: <1432225472-5186-1-git-send-email-r.krypa@samsung.com> <1432225472-5186-2-git-send-email-r.krypa@samsung.com> In-Reply-To: <1432225472-5186-2-git-send-email-r.krypa@samsung.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 5/21/2015 9:24 AM, Rafal Krypa wrote: > Use proper RCU functions and read locking in smackfs seq_operations. > > Smack gets away with not using proper RCU functions in smackfs, because > it never removes entries from these lists. But now one list will be > needed (with interface in smackfs) that will have both elements added and > removed to it. > This change will also help any future changes implementing removal of > unneeded entries from other Smack lists. > > The patch also fixes handling of pos argument in smk_seq_start and > smk_seq_next. This fixes a bug in case when smackfs is read with a small > buffer: > > Kernel panic - not syncing: Kernel mode fault at addr 0xfa0000011b > CPU: 0 PID: 1292 Comm: dd Not tainted 4.1.0-rc1-00012-g98179b8 #13 > Stack: > 00000003 0000000d 7ff39e48 7f69fd00 > 7ff39ce0 601ae4b0 7ff39d50 600e587b > 00000010 6039f690 7f69fd40 00612003 > Call Trace: > [<601ae4b0>] load2_seq_show+0x19/0x1d > [<600e587b>] seq_read+0x168/0x331 > [<600c5943>] __vfs_read+0x21/0x101 > [<601a595e>] ? security_file_permission+0xf8/0x105 > [<600c5ec6>] ? rw_verify_area+0x86/0xe2 > [<600c5fc3>] vfs_read+0xa1/0x14c > [<600c68e2>] SyS_read+0x57/0xa0 > [<6001da60>] handle_syscall+0x60/0x80 > [<6003087d>] userspace+0x442/0x548 > [<6001aa77>] ? interrupt_end+0x0/0x80 > [<6001daae>] ? copy_chunk_to_user+0x0/0x2b > [<6002cb6b>] ? save_registers+0x1f/0x39 > [<60032ef7>] ? arch_prctl+0xf5/0x170 > [<6001a92d>] fork_handler+0x85/0x87 > > Signed-off-by: Rafal Krypa Applied to https://github.com/cschaufler/smack-next.git#smack-for-4.2-stacked > --- > security/smack/smackfs.c | 52 ++++++++++++++++++++---------------------------- > 1 file changed, 22 insertions(+), 30 deletions(-) > > diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c > index 3e42426..e40dc48 100644 > --- a/security/smack/smackfs.c > +++ b/security/smack/smackfs.c > @@ -566,23 +566,17 @@ static void *smk_seq_start(struct seq_file *s, loff_t *pos, > struct list_head *head) > { > struct list_head *list; > + int i = *pos; > + > + rcu_read_lock(); > + for (list = rcu_dereference(list_next_rcu(head)); > + list != head; > + list = rcu_dereference(list_next_rcu(list))) { > + if (i-- == 0) > + return list; > + } > > - /* > - * This is 0 the first time through. > - */ > - if (s->index == 0) > - s->private = head; > - > - if (s->private == NULL) > - return NULL; > - > - list = s->private; > - if (list_empty(list)) > - return NULL; > - > - if (s->index == 0) > - return list->next; > - return list; > + return NULL; > } > > static void *smk_seq_next(struct seq_file *s, void *v, loff_t *pos, > @@ -590,17 +584,15 @@ static void *smk_seq_next(struct seq_file *s, void *v, loff_t *pos, > { > struct list_head *list = v; > > - if (list_is_last(list, head)) { > - s->private = NULL; > - return NULL; > - } > - s->private = list->next; > - return list->next; > + ++*pos; > + list = rcu_dereference(list_next_rcu(list)); > + > + return (list == head) ? NULL : list; > } > > static void smk_seq_stop(struct seq_file *s, void *v) > { > - /* No-op */ > + rcu_read_unlock(); > } > > static void smk_rule_show(struct seq_file *s, struct smack_rule *srp, int max) > @@ -660,7 +652,7 @@ static int load_seq_show(struct seq_file *s, void *v) > { > struct list_head *list = v; > struct smack_master_list *smlp = > - list_entry(list, struct smack_master_list, list); > + list_entry_rcu(list, struct smack_master_list, list); > > smk_rule_show(s, smlp->smk_rule, SMK_LABELLEN); > > @@ -808,7 +800,7 @@ static int cipso_seq_show(struct seq_file *s, void *v) > { > struct list_head *list = v; > struct smack_known *skp = > - list_entry(list, struct smack_known, list); > + list_entry_rcu(list, struct smack_known, list); > struct netlbl_lsm_catmap *cmp = skp->smk_netlabel.attr.mls.cat; > char sep = '/'; > int i; > @@ -999,7 +991,7 @@ static int cipso2_seq_show(struct seq_file *s, void *v) > { > struct list_head *list = v; > struct smack_known *skp = > - list_entry(list, struct smack_known, list); > + list_entry_rcu(list, struct smack_known, list); > struct netlbl_lsm_catmap *cmp = skp->smk_netlabel.attr.mls.cat; > char sep = '/'; > int i; > @@ -1083,7 +1075,7 @@ static int netlbladdr_seq_show(struct seq_file *s, void *v) > { > struct list_head *list = v; > struct smk_netlbladdr *skp = > - list_entry(list, struct smk_netlbladdr, list); > + list_entry_rcu(list, struct smk_netlbladdr, list); > unsigned char *hp = (char *) &skp->smk_host.sin_addr.s_addr; > int maskn; > u32 temp_mask = be32_to_cpu(skp->smk_mask.s_addr); > @@ -1917,7 +1909,7 @@ static int load_self_seq_show(struct seq_file *s, void *v) > { > struct list_head *list = v; > struct smack_rule *srp = > - list_entry(list, struct smack_rule, list); > + list_entry_rcu(list, struct smack_rule, list); > > smk_rule_show(s, srp, SMK_LABELLEN); > > @@ -2046,7 +2038,7 @@ static int load2_seq_show(struct seq_file *s, void *v) > { > struct list_head *list = v; > struct smack_master_list *smlp = > - list_entry(list, struct smack_master_list, list); > + list_entry_rcu(list, struct smack_master_list, list); > > smk_rule_show(s, smlp->smk_rule, SMK_LONGLABEL); > > @@ -2123,7 +2115,7 @@ static int load_self2_seq_show(struct seq_file *s, void *v) > { > struct list_head *list = v; > struct smack_rule *srp = > - list_entry(list, struct smack_rule, list); > + list_entry_rcu(list, struct smack_rule, list); > > smk_rule_show(s, srp, SMK_LONGLABEL); >