[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <556DFD1E.4060207@schaufler-ca.com>
Date: Tue, 02 Jun 2015 11:59:42 -0700
From: Casey Schaufler <casey@...aufler-ca.com>
To: Rafal Krypa <r.krypa@...sung.com>
CC: James Morris <james.l.morris@...cle.com>,
"Serge E. Hallyn" <serge@...lyn.com>,
linux-security-module@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] Smack: fix seq operations in smackfs
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 <r.krypa@...sung.com>
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);
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists