Smack: fix behavior of smack_inode_listsecurity
Subsystems (2)
PATCH 1/3 Smack: fix behavior of smack_inode_listsecurity
Security operation ->inode_listsecurity is used for generating list of available extended attributes for syscall listxattr. Currently it's used only in nfs4 or if filesystem doesn't provide i_op->listxattr.
The list is the set of NULL-terminated names, one after the other. This method must include zero byte at the and into result.
Also this function must return length even if string does not fit into output buffer or it is NULL, see similar method in selinux and man listxattr.
--- security/smack/smack_lsm.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
security/smack/smack_lsm.c +4 -5
@@ -1122,13 +1122,12 @@ static int smack_inode_getsecurity(const struct inode *inode,static int smack_inode_listsecurity (struct inode *inode,char *buffer,size_t buffer_size) { -int len =strlen (XATTR_NAME_SMACK ); +int len =sizeof (XATTR_NAME_SMACK ); -if (buffer !=NULL && len <= buffer_size) { +if (buffer !=NULL && len <= buffer_size)memcpy (buffer,XATTR_NAME_SMACK , len); -return len; - } -return -EINVAL ; + +return len; }/**
PATCH 2/3 Smack: handle zero-length security labels without panic
Zero-length security labels are invalid but kernel should handle them.
This patch fixes kernel panic after setting zero-length security labels:
# attr -S -s "SMACK64" -V "" file
And after writing zero-length string into smackfs files syslog and onlycp:
# python -c 'import os; os.write(1, "")' > /smack/syslog
The problem is caused by brain-damaged logic in function smk_parse_smack() which takes pointer to buffer and its length but if length below or equal zero it thinks that the buffer is zero-terminated. Unfortunately callers of this function are widely used and proper fix requires serious refactoring.
---
Example:
[ 28.063935] BUG: unable to handle kernel NULL pointer dereference at (null) [ 28.064623] IP: [<ffffffff812ea780>] strlen+0x0/0x30 [ 28.064623] PGD 29a77067 PUD 2972d067 PMD 0 [ 28.064623] Oops: 0000 [#1] SMP [ 28.064623] Modules linked in: [ 28.064623] CPU: 0 PID: 824 Comm: attr Not tainted 3.16.0+ #6 [ 28.064623] task: ffff880029bd88c0 ti: ffff8800297d0000 task.ti: ffff8800297d0000 [ 28.064623] RIP: 0010:[<ffffffff812ea780>] [<ffffffff812ea780>] strlen+0x0/0x30 [ 28.064623] RSP: 0018:ffff8800297d3ca8 EFLAGS: 00010246 [ 28.064623] RAX: 0000000000000001 RBX: ffff880029c096c0 RCX: 0000000000000001 [ 28.064623] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000 [ 28.064623] RBP: ffff8800297d3cd0 R08: ffffffff81e3d940 R09: ffff88002a3e8588 [ 28.064623] R10: 8080808080808080 R11: 0000000000000000 R12: 0000000000000000 [ 28.064623] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000 [ 28.064623] FS: 00007effe391a700(0000) GS:ffff88002a400000(0000) knlGS:0000000000000000 [ 28.064623] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 28.064623] CR2: 0000000000000000 CR3: 00000000297a1000 CR4: 00000000001406b0 [ 28.064623] Stack: [ 28.064623] ffffffff812a6fa5 ffff880029c096c0 0000000000000001 0000000000000000 [ 28.064623] 0000000000000000 ffff8800297d3cf8 ffffffff812a70ae ffff880029c096c0 [ 28.064623] 0000000000000001 0000000000000000 ffff8800297d3d88 ffffffff812a64dc [ 28.064623] Call Trace: [ 28.064623] [<ffffffff812a6fa5>] ? smk_parse_smack+0xe5/0x110 [ 28.064623] [<ffffffff812a70ae>] smk_import_entry+0xe/0x130 [ 28.064623] [<ffffffff812a64dc>] smack_inode_setxattr+0x11c/0x280 [ 28.064623] [<ffffffff8116a767>] ? lookup_fast+0x177/0x2e0 [ 28.064623] [<ffffffff81177001>] ? dput+0x21/0x1a0 [ 28.064623] [<ffffffff812a0eeb>] security_inode_setxattr+0x1b/0x30 [ 28.064623] [<ffffffff8118488f>] vfs_setxattr+0x6f/0xb0 [ 28.064623] [<ffffffff81184a76>] setxattr+0x1a6/0x1f0 [ 28.064623] [<ffffffff8116af6d>] ? final_putname+0x1d/0x40 [ 28.064623] [<ffffffff8116b1c4>] ? putname+0x24/0x40 [ 28.064623] [<ffffffff8117144a>] ? user_path_at_empty+0x5a/0xa0 [ 28.064623] [<ffffffff81162b84>] ? __sb_start_write+0x44/0xe0 [ 28.064623] [<ffffffff8113cd21>] ? do_brk+0x241/0x320 [ 28.064623] [<ffffffff81184d97>] SyS_lsetxattr+0x87/0xe0 [ 28.064623] [<ffffffff81806f69>] system_call_fastpath+0x16/0x1b [ 28.064623] Code: 89 f8 48 89 e5 f6 82 40 3e a6 81 20 74 15 0f 1f 44 00 00 48 83 c0 01 0f b6 10 f6 82 40 3e a6 81 20 75 f0 5d c3 66 0f 1f 44 00 00 <80> 3f 00 55 48 89 e5 74 15 48 89 f8 0f 1f 40 00 48 83 c0 01 80 [ 28.064623] RIP [<ffffffff812ea780>] strlen+0x0/0x30 [ 28.064623] RSP <ffff8800297d3ca8> [ 28.064623] CR2: 0000000000000000 [ 28.093561] ---[ end trace de1055429a98a5be ]--- --- security/smack/smack_lsm.c | 2 +- security/smack/smackfs.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)
security/smack/smack_lsm.c +1 -1
@@ -923,7 +923,7 @@ static int smack_inode_setxattr(struct dentry *dentry, const char *name, rc = -EPERM ;if (rc ==0 && check_import) { - skp =smk_import_entry (value, size); + skp = size ?smk_import_entry (value, size) :NULL ;if (skp ==NULL || (check_star && (skp == &smack_known_star || skp == &smack_known_web))) rc = -EINVAL ;
security/smack/smackfs.c +2 -2
@@ -1677,7 +1677,7 @@ static ssize_t smk_write_onlycap(struct file *file, const char __user *buf,if (smack_onlycap !=NULL && smack_onlycap != skp)return -EPERM ; - data =kzalloc (count,GFP_KERNEL ); + data =kzalloc (count +1 ,GFP_KERNEL );if (data ==NULL )return -ENOMEM ;
@@ -2228,7 +2228,7 @@ static ssize_t smk_write_syslog(struct file *file, const char __user *buf,if (!smack_privileged (CAP_MAC_ADMIN ))return -EPERM ; - data =kzalloc (count,GFP_KERNEL ); + data =kzalloc (count +1 ,GFP_KERNEL );if (data ==NULL )return -ENOMEM ;
Re: [PATCH 2/3] Smack: handle zero-length security labels without panic
Quoted 6 lines · Konstantin Khlebnikov
Zero-length security labels are invalid but kernel should handle them.
This patch fixes kernel panic after setting zero-length security labels:
# attr -S -s "SMACK64" -V "" fileAnd after writing zero-length string into smackfs files syslog and onlycp:
Quoted 8 lines
# python -c 'import os; os.write(1, "")' > /smack/syslogThe problem is caused by brain-damaged logic in function
smk_parse_smack()which takes pointer to buffer and its length but if length below or equal zero it thinks that the buffer is zero-terminated. Unfortunately callers of this function are widely used and proper fix requires serious refactoring.Signed-off-by Konstantin Khlebnikov <[email protected]>
Quoted 79 lines
---
Example:
[ 28.063935] BUG: unable to handle kernel NULL pointer dereference at (null) [ 28.064623] IP: [<ffffffff812ea780>] strlen+0x0/0x30 [ 28.064623] PGD 29a77067 PUD 2972d067 PMD 0 [ 28.064623] Oops: 0000 [#1] SMP [ 28.064623] Modules linked in: [ 28.064623] CPU: 0 PID: 824 Comm: attr Not tainted 3.16.0+ #6 [ 28.064623] task: ffff880029bd88c0 ti: ffff8800297d0000 task.ti: ffff8800297d0000 [ 28.064623] RIP: 0010:[<ffffffff812ea780>] [<ffffffff812ea780>] strlen+0x0/0x30 [ 28.064623] RSP: 0018:ffff8800297d3ca8 EFLAGS: 00010246 [ 28.064623] RAX: 0000000000000001 RBX: ffff880029c096c0 RCX: 0000000000000001 [ 28.064623] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000 [ 28.064623] RBP: ffff8800297d3cd0 R08: ffffffff81e3d940 R09: ffff88002a3e8588 [ 28.064623] R10: 8080808080808080 R11: 0000000000000000 R12: 0000000000000000 [ 28.064623] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000 [ 28.064623] FS: 00007effe391a700(0000) GS:ffff88002a400000(0000) knlGS:0000000000000000 [ 28.064623] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 28.064623] CR2: 0000000000000000 CR3: 00000000297a1000 CR4: 00000000001406b0 [ 28.064623] Stack: [ 28.064623] ffffffff812a6fa5 ffff880029c096c0 0000000000000001 0000000000000000 [ 28.064623] 0000000000000000 ffff8800297d3cf8 ffffffff812a70ae ffff880029c096c0 [ 28.064623] 0000000000000001 0000000000000000 ffff8800297d3d88 ffffffff812a64dc [ 28.064623] Call Trace: [ 28.064623] [<ffffffff812a6fa5>] ? smk_parse_smack+0xe5/0x110 [ 28.064623] [<ffffffff812a70ae>] smk_import_entry+0xe/0x130 [ 28.064623] [<ffffffff812a64dc>] smack_inode_setxattr+0x11c/0x280 [ 28.064623] [<ffffffff8116a767>] ? lookup_fast+0x177/0x2e0 [ 28.064623] [<ffffffff81177001>] ? dput+0x21/0x1a0 [ 28.064623] [<ffffffff812a0eeb>] security_inode_setxattr+0x1b/0x30 [ 28.064623] [<ffffffff8118488f>] vfs_setxattr+0x6f/0xb0 [ 28.064623] [<ffffffff81184a76>] setxattr+0x1a6/0x1f0 [ 28.064623] [<ffffffff8116af6d>] ? final_putname+0x1d/0x40 [ 28.064623] [<ffffffff8116b1c4>] ? putname+0x24/0x40 [ 28.064623] [<ffffffff8117144a>] ? user_path_at_empty+0x5a/0xa0 [ 28.064623] [<ffffffff81162b84>] ? __sb_start_write+0x44/0xe0 [ 28.064623] [<ffffffff8113cd21>] ? do_brk+0x241/0x320 [ 28.064623] [<ffffffff81184d97>] SyS_lsetxattr+0x87/0xe0 [ 28.064623] [<ffffffff81806f69>] system_call_fastpath+0x16/0x1b [ 28.064623] Code: 89 f8 48 89 e5 f6 82 40 3e a6 81 20 74 15 0f 1f 44 00 00 48 83 c0 01 0f b6 10 f6 82 40 3e a6 81 20 75 f0 5d c3 66 0f 1f 44 00 00 <80> 3f 00 55 48 89 e5 74 15 48 89 f8 0f 1f 40 00 48 83 c0 01 80 [ 28.064623] RIP [<ffffffff812ea780>] strlen+0x0/0x30 [ 28.064623] RSP <ffff8800297d3ca8> [ 28.064623] CR2: 0000000000000000 [ 28.093561] ---[ end trace de1055429a98a5be ]--- --- security/smack/smack_lsm.c | 2 +- security/smack/smackfs.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)security/smack/smack_lsm.c +1
-1@@ -923,7 +923,7 @@ static int smack_inode_setxattr(struct dentry *dentry, const char *name, rc = -EPERM ;if (rc ==0 && check_import) { - skp =smk_import_entry (value, size); + skp = size ?smk_import_entry (value, size) :NULL ;if (skp ==NULL || (check_star && (skp == &smack_known_star || skp == &smack_known_web))) rc = -EINVAL ;security/smack/smackfs.c +1
-1@@ -1677,7 +1677,7 @@ static ssize_t smk_write_onlycap(struct file *file, const char __user *buf,if (smack_onlycap !=NULL && smack_onlycap != skp)return -EPERM ; - data =kzalloc (count,GFP_KERNEL ); + data =kzalloc (count +1 ,GFP_KERNEL );if (data ==NULL )return -ENOMEM ;
security/smack/smackfs.c static ssize_t smk_write_syslog(struct file *file, const char __user *buf, · line 2230
Quoted 3 lines
security/smack/smackfs.c +0
-0@@ -2228,7 +2228,7 @@ static ssize_t smk_write_syslog(struct file *file, const char __user *buf,if (!smack_privileged (CAP_MAC_ADMIN ))return -EPERM ;
How about putting a comment here explaining that this way if len is 0 you make sure to have '\0' to avoid parsing problems?
Quoted 5 lines
security/smack/smackfs.c +1
-1- data =kzalloc (count,GFP_KERNEL ); + data =kzalloc (count +1 ,GFP_KERNEL );if (data ==NULL )return -ENOMEM ;
Re: [PATCH 2/3] Smack: handle zero-length security labels without panic
Quoted 96 lines · Serge E. Hallyn
Quoted 6 lines · Konstantin Khlebnikov
Zero-length security labels are invalid but kernel should handle them.
This patch fixes kernel panic after setting zero-length security labels:
# attr -S -s "SMACK64" -V "" fileAnd after writing zero-length string into smackfs files syslog and onlycp:
Quoted 8 lines
# python -c 'import os; os.write(1, "")' > /smack/syslogThe problem is caused by brain-damaged logic in function
smk_parse_smack()which takes pointer to buffer and its length but if length below or equal zero it thinks that the buffer is zero-terminated. Unfortunately callers of this function are widely used and proper fix requires serious refactoring.Signed-off-by Konstantin Khlebnikov <[email protected]>Acked-by Serge Hallyn <[email protected]>Quoted 78 lines
---
Example:
[ 28.063935] BUG: unable to handle kernel NULL pointer dereference at (null) [ 28.064623] IP: [<ffffffff812ea780>] strlen+0x0/0x30 [ 28.064623] PGD 29a77067 PUD 2972d067 PMD 0 [ 28.064623] Oops: 0000 [#1] SMP [ 28.064623] Modules linked in: [ 28.064623] CPU: 0 PID: 824 Comm: attr Not tainted 3.16.0+ #6 [ 28.064623] task: ffff880029bd88c0 ti: ffff8800297d0000 task.ti: ffff8800297d0000 [ 28.064623] RIP: 0010:[<ffffffff812ea780>] [<ffffffff812ea780>] strlen+0x0/0x30 [ 28.064623] RSP: 0018:ffff8800297d3ca8 EFLAGS: 00010246 [ 28.064623] RAX: 0000000000000001 RBX: ffff880029c096c0 RCX: 0000000000000001 [ 28.064623] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000 [ 28.064623] RBP: ffff8800297d3cd0 R08: ffffffff81e3d940 R09: ffff88002a3e8588 [ 28.064623] R10: 8080808080808080 R11: 0000000000000000 R12: 0000000000000000 [ 28.064623] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000 [ 28.064623] FS: 00007effe391a700(0000) GS:ffff88002a400000(0000) knlGS:0000000000000000 [ 28.064623] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 28.064623] CR2: 0000000000000000 CR3: 00000000297a1000 CR4: 00000000001406b0 [ 28.064623] Stack: [ 28.064623] ffffffff812a6fa5 ffff880029c096c0 0000000000000001 0000000000000000 [ 28.064623] 0000000000000000 ffff8800297d3cf8 ffffffff812a70ae ffff880029c096c0 [ 28.064623] 0000000000000001 0000000000000000 ffff8800297d3d88 ffffffff812a64dc [ 28.064623] Call Trace: [ 28.064623] [<ffffffff812a6fa5>] ? smk_parse_smack+0xe5/0x110 [ 28.064623] [<ffffffff812a70ae>] smk_import_entry+0xe/0x130 [ 28.064623] [<ffffffff812a64dc>] smack_inode_setxattr+0x11c/0x280 [ 28.064623] [<ffffffff8116a767>] ? lookup_fast+0x177/0x2e0 [ 28.064623] [<ffffffff81177001>] ? dput+0x21/0x1a0 [ 28.064623] [<ffffffff812a0eeb>] security_inode_setxattr+0x1b/0x30 [ 28.064623] [<ffffffff8118488f>] vfs_setxattr+0x6f/0xb0 [ 28.064623] [<ffffffff81184a76>] setxattr+0x1a6/0x1f0 [ 28.064623] [<ffffffff8116af6d>] ? final_putname+0x1d/0x40 [ 28.064623] [<ffffffff8116b1c4>] ? putname+0x24/0x40 [ 28.064623] [<ffffffff8117144a>] ? user_path_at_empty+0x5a/0xa0 [ 28.064623] [<ffffffff81162b84>] ? __sb_start_write+0x44/0xe0 [ 28.064623] [<ffffffff8113cd21>] ? do_brk+0x241/0x320 [ 28.064623] [<ffffffff81184d97>] SyS_lsetxattr+0x87/0xe0 [ 28.064623] [<ffffffff81806f69>] system_call_fastpath+0x16/0x1b [ 28.064623] Code: 89 f8 48 89 e5 f6 82 40 3e a6 81 20 74 15 0f 1f 44 00 00 48 83 c0 01 0f b6 10 f6 82 40 3e a6 81 20 75 f0 5d c3 66 0f 1f 44 00 00 <80> 3f 00 55 48 89 e5 74 15 48 89 f8 0f 1f 40 00 48 83 c0 01 80 [ 28.064623] RIP [<ffffffff812ea780>] strlen+0x0/0x30 [ 28.064623] RSP <ffff8800297d3ca8> [ 28.064623] CR2: 0000000000000000 [ 28.093561] ---[ end trace de1055429a98a5be ]--- --- security/smack/smack_lsm.c | 2 +- security/smack/smackfs.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)security/smack/smack_lsm.c +1
-1@@ -923,7 +923,7 @@ static int smack_inode_setxattr(struct dentry *dentry, const char *name, rc = -EPERM ;if (rc ==0 && check_import) { - skp =smk_import_entry (value, size); + skp = size ?smk_import_entry (value, size) :NULL ;if (skp ==NULL || (check_star && (skp == &smack_known_star || skp == &smack_known_web))) rc = -EINVAL ;security/smack/smackfs.c +1
-1@@ -1677,7 +1677,7 @@ static ssize_t smk_write_onlycap(struct file *file, const char __user *buf,if (smack_onlycap !=NULL && smack_onlycap != skp)return -EPERM ; - data =kzalloc (count,GFP_KERNEL ); + data =kzalloc (count +1 ,GFP_KERNEL );if (data ==NULL )return -ENOMEM ;
Quoted 7 lines
security/smack/smackfs.c static ssize_t smk_write_syslog(struct file *file, const char __user *buf, · line 2230
Quoted 4 lines
security/smack/smackfs.c +0
-0@@ -2228,7 +2228,7 @@ static ssize_t smk_write_syslog(struct file *file, const char __user *buf,if (!smack_privileged (CAP_MAC_ADMIN ))return -EPERM ;How about putting a comment here explaining that this way if len is 0 you make sure to have '\0' to avoid parsing problems?
There are plenty of these comments. That design is uncommon and extremely dangerous,
it cannot be neutralized just by a comment.
Probably magic length '0' might be replaced with magic '-1' which is much safer.
Quoted 11 lines
security/smack/smackfs.c static ssize_t smk_write_onlycap(struct file *file, const char __user *buf, · line 1683
Quoted 6 lines
security/smack/smackfs.c +1
-1- data =kzalloc (count,GFP_KERNEL ); + data =kzalloc (count +1 ,GFP_KERNEL );if (data ==NULL )return -ENOMEM ;--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Serge E. Hallyn
maintainerRe: [PATCH 2/3] Smack: handle zero-length security labels without panic
Quoted 105 lines · Konstantin Khlebnikov
Quoted 96 lines · Serge E. Hallyn
Quoted 6 lines · Konstantin Khlebnikov
Zero-length security labels are invalid but kernel should handle them.
This patch fixes kernel panic after setting zero-length security labels:
# attr -S -s "SMACK64" -V "" fileAnd after writing zero-length string into smackfs files syslog and onlycp:
Quoted 8 lines
# python -c 'import os; os.write(1, "")' > /smack/syslogThe problem is caused by brain-damaged logic in function
smk_parse_smack()which takes pointer to buffer and its length but if length below or equal zero it thinks that the buffer is zero-terminated. Unfortunately callers of this function are widely used and proper fix requires serious refactoring.Signed-off-by Konstantin Khlebnikov <[email protected]>Acked-by Serge Hallyn <[email protected]>Quoted 78 lines
---
Example:
[ 28.063935] BUG: unable to handle kernel NULL pointer dereference at (null) [ 28.064623] IP: [<ffffffff812ea780>] strlen+0x0/0x30 [ 28.064623] PGD 29a77067 PUD 2972d067 PMD 0 [ 28.064623] Oops: 0000 [#1] SMP [ 28.064623] Modules linked in: [ 28.064623] CPU: 0 PID: 824 Comm: attr Not tainted 3.16.0+ #6 [ 28.064623] task: ffff880029bd88c0 ti: ffff8800297d0000 task.ti: ffff8800297d0000 [ 28.064623] RIP: 0010:[<ffffffff812ea780>] [<ffffffff812ea780>] strlen+0x0/0x30 [ 28.064623] RSP: 0018:ffff8800297d3ca8 EFLAGS: 00010246 [ 28.064623] RAX: 0000000000000001 RBX: ffff880029c096c0 RCX: 0000000000000001 [ 28.064623] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000 [ 28.064623] RBP: ffff8800297d3cd0 R08: ffffffff81e3d940 R09: ffff88002a3e8588 [ 28.064623] R10: 8080808080808080 R11: 0000000000000000 R12: 0000000000000000 [ 28.064623] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000 [ 28.064623] FS: 00007effe391a700(0000) GS:ffff88002a400000(0000) knlGS:0000000000000000 [ 28.064623] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 28.064623] CR2: 0000000000000000 CR3: 00000000297a1000 CR4: 00000000001406b0 [ 28.064623] Stack: [ 28.064623] ffffffff812a6fa5 ffff880029c096c0 0000000000000001 0000000000000000 [ 28.064623] 0000000000000000 ffff8800297d3cf8 ffffffff812a70ae ffff880029c096c0 [ 28.064623] 0000000000000001 0000000000000000 ffff8800297d3d88 ffffffff812a64dc [ 28.064623] Call Trace: [ 28.064623] [<ffffffff812a6fa5>] ? smk_parse_smack+0xe5/0x110 [ 28.064623] [<ffffffff812a70ae>] smk_import_entry+0xe/0x130 [ 28.064623] [<ffffffff812a64dc>] smack_inode_setxattr+0x11c/0x280 [ 28.064623] [<ffffffff8116a767>] ? lookup_fast+0x177/0x2e0 [ 28.064623] [<ffffffff81177001>] ? dput+0x21/0x1a0 [ 28.064623] [<ffffffff812a0eeb>] security_inode_setxattr+0x1b/0x30 [ 28.064623] [<ffffffff8118488f>] vfs_setxattr+0x6f/0xb0 [ 28.064623] [<ffffffff81184a76>] setxattr+0x1a6/0x1f0 [ 28.064623] [<ffffffff8116af6d>] ? final_putname+0x1d/0x40 [ 28.064623] [<ffffffff8116b1c4>] ? putname+0x24/0x40 [ 28.064623] [<ffffffff8117144a>] ? user_path_at_empty+0x5a/0xa0 [ 28.064623] [<ffffffff81162b84>] ? __sb_start_write+0x44/0xe0 [ 28.064623] [<ffffffff8113cd21>] ? do_brk+0x241/0x320 [ 28.064623] [<ffffffff81184d97>] SyS_lsetxattr+0x87/0xe0 [ 28.064623] [<ffffffff81806f69>] system_call_fastpath+0x16/0x1b [ 28.064623] Code: 89 f8 48 89 e5 f6 82 40 3e a6 81 20 74 15 0f 1f 44 00 00 48 83 c0 01 0f b6 10 f6 82 40 3e a6 81 20 75 f0 5d c3 66 0f 1f 44 00 00 <80> 3f 00 55 48 89 e5 74 15 48 89 f8 0f 1f 40 00 48 83 c0 01 80 [ 28.064623] RIP [<ffffffff812ea780>] strlen+0x0/0x30 [ 28.064623] RSP <ffff8800297d3ca8> [ 28.064623] CR2: 0000000000000000 [ 28.093561] ---[ end trace de1055429a98a5be ]--- --- security/smack/smack_lsm.c | 2 +- security/smack/smackfs.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)security/smack/smack_lsm.c +1
-1@@ -923,7 +923,7 @@ static int smack_inode_setxattr(struct dentry *dentry, const char *name, rc = -EPERM ;if (rc ==0 && check_import) { - skp =smk_import_entry (value, size); + skp = size ?smk_import_entry (value, size) :NULL ;if (skp ==NULL || (check_star && (skp == &smack_known_star || skp == &smack_known_web))) rc = -EINVAL ;security/smack/smackfs.c +1
-1@@ -1677,7 +1677,7 @@ static ssize_t smk_write_onlycap(struct file *file, const char __user *buf,if (smack_onlycap !=NULL && smack_onlycap != skp)return -EPERM ; - data =kzalloc (count,GFP_KERNEL ); + data =kzalloc (count +1 ,GFP_KERNEL );if (data ==NULL )return -ENOMEM ;Quoted 7 lines
security/smack/smackfs.c static ssize_t smk_write_syslog(struct file *file, const char __user *buf, · line 2230
Quoted 4 lines
security/smack/smackfs.c +0
-0@@ -2228,7 +2228,7 @@ static ssize_t smk_write_syslog(struct file *file, const char __user *buf,if (!smack_privileged (CAP_MAC_ADMIN ))return -EPERM ;How about putting a comment here explaining that this way if len is 0 you make sure to have '\0' to avoid parsing problems?
Quoted 1 line
There are plenty of these comments. That design is uncommon and
Exactly why it should be documented.
Quoted 2 lines
extremely dangerous,
it cannot be neutralized just by a comment.
It's not about neutralizing it. It's about down the road, while you're on leave for a few months, someone saying "say that +1 seems unnecessary, I'll just remove it."
Quoted 1 line
Probably magic length '0' might be replaced with magic '-1' which is much safer.
Could be.
Quoted 11 lines
Quoted 11 lines
security/smack/smackfs.c static ssize_t smk_write_onlycap(struct file *file, const char __user *buf, · line 1683
Quoted 6 lines
security/smack/smackfs.c +1
-1- data =kzalloc (count,GFP_KERNEL ); + data =kzalloc (count +1 ,GFP_KERNEL );if (data ==NULL )return -ENOMEM ;--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Casey Schaufler
maintainerRe: [PATCH 2/3] Smack: handle zero-length security labels without panic
Quoted 96 lines · Serge E. Hallyn
Quoted 95 lines · Konstantin Khlebnikov
Quoted 94 lines · Serge E. Hallyn
Quoted 6 lines · Konstantin Khlebnikov
Zero-length security labels are invalid but kernel should handle them.
This patch fixes kernel panic after setting zero-length security labels:
# attr -S -s "SMACK64" -V "" fileAnd after writing zero-length string into smackfs files syslog and onlycp:
Quoted 8 lines
# python -c 'import os; os.write(1, "")' > /smack/syslogThe problem is caused by brain-damaged logic in function
smk_parse_smack()which takes pointer to buffer and its length but if length below or equal zero it thinks that the buffer is zero-terminated. Unfortunately callers of this function are widely used and proper fix requires serious refactoring.Signed-off-by Konstantin Khlebnikov <[email protected]>Acked-by Serge Hallyn <[email protected]>Quoted 77 lines
---
Example:
[ 28.063935] BUG: unable to handle kernel NULL pointer dereference at (null) [ 28.064623] IP: [<ffffffff812ea780>] strlen+0x0/0x30 [ 28.064623] PGD 29a77067 PUD 2972d067 PMD 0 [ 28.064623] Oops: 0000 [#1] SMP [ 28.064623] Modules linked in: [ 28.064623] CPU: 0 PID: 824 Comm: attr Not tainted 3.16.0+ #6 [ 28.064623] task: ffff880029bd88c0 ti: ffff8800297d0000 task.ti: ffff8800297d0000 [ 28.064623] RIP: 0010:[<ffffffff812ea780>] [<ffffffff812ea780>] strlen+0x0/0x30 [ 28.064623] RSP: 0018:ffff8800297d3ca8 EFLAGS: 00010246 [ 28.064623] RAX: 0000000000000001 RBX: ffff880029c096c0 RCX: 0000000000000001 [ 28.064623] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000 [ 28.064623] RBP: ffff8800297d3cd0 R08: ffffffff81e3d940 R09: ffff88002a3e8588 [ 28.064623] R10: 8080808080808080 R11: 0000000000000000 R12: 0000000000000000 [ 28.064623] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000 [ 28.064623] FS: 00007effe391a700(0000) GS:ffff88002a400000(0000) knlGS:0000000000000000 [ 28.064623] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 28.064623] CR2: 0000000000000000 CR3: 00000000297a1000 CR4: 00000000001406b0 [ 28.064623] Stack: [ 28.064623] ffffffff812a6fa5 ffff880029c096c0 0000000000000001 0000000000000000 [ 28.064623] 0000000000000000 ffff8800297d3cf8 ffffffff812a70ae ffff880029c096c0 [ 28.064623] 0000000000000001 0000000000000000 ffff8800297d3d88 ffffffff812a64dc [ 28.064623] Call Trace: [ 28.064623] [<ffffffff812a6fa5>] ? smk_parse_smack+0xe5/0x110 [ 28.064623] [<ffffffff812a70ae>] smk_import_entry+0xe/0x130 [ 28.064623] [<ffffffff812a64dc>] smack_inode_setxattr+0x11c/0x280 [ 28.064623] [<ffffffff8116a767>] ? lookup_fast+0x177/0x2e0 [ 28.064623] [<ffffffff81177001>] ? dput+0x21/0x1a0 [ 28.064623] [<ffffffff812a0eeb>] security_inode_setxattr+0x1b/0x30 [ 28.064623] [<ffffffff8118488f>] vfs_setxattr+0x6f/0xb0 [ 28.064623] [<ffffffff81184a76>] setxattr+0x1a6/0x1f0 [ 28.064623] [<ffffffff8116af6d>] ? final_putname+0x1d/0x40 [ 28.064623] [<ffffffff8116b1c4>] ? putname+0x24/0x40 [ 28.064623] [<ffffffff8117144a>] ? user_path_at_empty+0x5a/0xa0 [ 28.064623] [<ffffffff81162b84>] ? __sb_start_write+0x44/0xe0 [ 28.064623] [<ffffffff8113cd21>] ? do_brk+0x241/0x320 [ 28.064623] [<ffffffff81184d97>] SyS_lsetxattr+0x87/0xe0 [ 28.064623] [<ffffffff81806f69>] system_call_fastpath+0x16/0x1b [ 28.064623] Code: 89 f8 48 89 e5 f6 82 40 3e a6 81 20 74 15 0f 1f 44 00 00 48 83 c0 01 0f b6 10 f6 82 40 3e a6 81 20 75 f0 5d c3 66 0f 1f 44 00 00 <80> 3f 00 55 48 89 e5 74 15 48 89 f8 0f 1f 40 00 48 83 c0 01 80 [ 28.064623] RIP [<ffffffff812ea780>] strlen+0x0/0x30 [ 28.064623] RSP <ffff8800297d3ca8> [ 28.064623] CR2: 0000000000000000 [ 28.093561] ---[ end trace de1055429a98a5be ]--- --- security/smack/smack_lsm.c | 2 +- security/smack/smackfs.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)security/smack/smack_lsm.c +1
-1@@ -923,7 +923,7 @@ static int smack_inode_setxattr(struct dentry *dentry, const char *name, rc = -EPERM ;if (rc ==0 && check_import) { - skp =smk_import_entry (value, size); + skp = size ?smk_import_entry (value, size) :NULL ;if (skp ==NULL || (check_star && (skp == &smack_known_star || skp == &smack_known_web))) rc = -EINVAL ;security/smack/smackfs.c +1
-1@@ -1677,7 +1677,7 @@ static ssize_t smk_write_onlycap(struct file *file, const char __user *buf,if (smack_onlycap !=NULL && smack_onlycap != skp)return -EPERM ; - data =kzalloc (count,GFP_KERNEL ); + data =kzalloc (count +1 ,GFP_KERNEL );if (data ==NULL )return -ENOMEM ;
Quoted 8 lines
Quoted 7 lines
Quoted 6 lines
security/smack/smackfs.c static ssize_t smk_write_syslog(struct file *file, const char __user *buf, · line 2230
Quoted 4 lines
security/smack/smackfs.c +0
-0@@ -2228,7 +2228,7 @@ static ssize_t smk_write_syslog(struct file *file, const char __user *buf,if (!smack_privileged (CAP_MAC_ADMIN ))return -EPERM ;How about putting a comment here explaining that this way if len is 0 you make sure to have '\0' to avoid parsing problems?
There are plenty of these comments. That design is uncommon and
Exactly why it should be documented.
The code was much simpler when I decided that a 0 length could safely indicate that the string was known to be terminated. Several other people have been in the code since, and it got crufted up in the name of performance. Oh well.Thank you for taking the time to provide a patch. I appreciate your efforts.
Quoted 5 lines
Quoted 3 lines
extremely dangerous,
it cannot be neutralized just by a comment.It's not about neutralizing it. It's about down the road, while you're on leave for a few months, someone saying "say that +1 seems unnecessary, I'll just remove it."
I'd rather have a pass through the entirety of the label import code. Some of the performance optimizations were done in a way that, while it seemed like a good idea at the time, didn't necessarily demonstrate a through understanding of what the surrounding code was up to.
Quoted 3 lines
Quoted 2 lines
Probably magic length '0' might be replaced with magic '-1' which is much safer.
Could be.
Not a bad idea.
Quoted 16 lines
Quoted 11 lines
Quoted 11 lines
security/smack/smackfs.c static ssize_t smk_write_onlycap(struct file *file, const char __user *buf, · line 1683
Quoted 6 lines
security/smack/smackfs.c +1
-1- data =kzalloc (count,GFP_KERNEL ); + data =kzalloc (count +1 ,GFP_KERNEL );if (data ==NULL )return -ENOMEM ;--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] Smack: handle zero-length security labels without panic
In reply to Konstantin KhlebnikovQuoted 6 lines · Konstantin Khlebnikov
Zero-length security labels are invalid but kernel should handle them.
This patch fixes kernel panic after setting zero-length security labels:
# attr -S -s "SMACK64" -V "" fileAnd after writing zero-length string into smackfs files syslog and onlycp:
Quoted 8 lines
# python -c 'import os; os.write(1, "")' > /smack/syslogThe problem is caused by brain-damaged logic in function
smk_parse_smack()which takes pointer to buffer and its length but if length below or equal zero it thinks that the buffer is zero-terminated. Unfortunately callers of this function are widely used and proper fix requires serious refactoring.Signed-off-by Konstantin Khlebnikov <[email protected]>
Applied to git://git.gitorious.org/smack-next/kernel.git#smack-for-3.18
Quoted 94 lines
---
Example:
[ 28.063935] BUG: unable to handle kernel NULL pointer dereference at (null) [ 28.064623] IP: [<ffffffff812ea780>] strlen+0x0/0x30 [ 28.064623] PGD 29a77067 PUD 2972d067 PMD 0 [ 28.064623] Oops: 0000 [#1] SMP [ 28.064623] Modules linked in: [ 28.064623] CPU: 0 PID: 824 Comm: attr Not tainted 3.16.0+ #6 [ 28.064623] task: ffff880029bd88c0 ti: ffff8800297d0000 task.ti: ffff8800297d0000 [ 28.064623] RIP: 0010:[<ffffffff812ea780>] [<ffffffff812ea780>] strlen+0x0/0x30 [ 28.064623] RSP: 0018:ffff8800297d3ca8 EFLAGS: 00010246 [ 28.064623] RAX: 0000000000000001 RBX: ffff880029c096c0 RCX: 0000000000000001 [ 28.064623] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000 [ 28.064623] RBP: ffff8800297d3cd0 R08: ffffffff81e3d940 R09: ffff88002a3e8588 [ 28.064623] R10: 8080808080808080 R11: 0000000000000000 R12: 0000000000000000 [ 28.064623] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000 [ 28.064623] FS: 00007effe391a700(0000) GS:ffff88002a400000(0000) knlGS:0000000000000000 [ 28.064623] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 28.064623] CR2: 0000000000000000 CR3: 00000000297a1000 CR4: 00000000001406b0 [ 28.064623] Stack: [ 28.064623] ffffffff812a6fa5 ffff880029c096c0 0000000000000001 0000000000000000 [ 28.064623] 0000000000000000 ffff8800297d3cf8 ffffffff812a70ae ffff880029c096c0 [ 28.064623] 0000000000000001 0000000000000000 ffff8800297d3d88 ffffffff812a64dc [ 28.064623] Call Trace: [ 28.064623] [<ffffffff812a6fa5>] ? smk_parse_smack+0xe5/0x110 [ 28.064623] [<ffffffff812a70ae>] smk_import_entry+0xe/0x130 [ 28.064623] [<ffffffff812a64dc>] smack_inode_setxattr+0x11c/0x280 [ 28.064623] [<ffffffff8116a767>] ? lookup_fast+0x177/0x2e0 [ 28.064623] [<ffffffff81177001>] ? dput+0x21/0x1a0 [ 28.064623] [<ffffffff812a0eeb>] security_inode_setxattr+0x1b/0x30 [ 28.064623] [<ffffffff8118488f>] vfs_setxattr+0x6f/0xb0 [ 28.064623] [<ffffffff81184a76>] setxattr+0x1a6/0x1f0 [ 28.064623] [<ffffffff8116af6d>] ? final_putname+0x1d/0x40 [ 28.064623] [<ffffffff8116b1c4>] ? putname+0x24/0x40 [ 28.064623] [<ffffffff8117144a>] ? user_path_at_empty+0x5a/0xa0 [ 28.064623] [<ffffffff81162b84>] ? __sb_start_write+0x44/0xe0 [ 28.064623] [<ffffffff8113cd21>] ? do_brk+0x241/0x320 [ 28.064623] [<ffffffff81184d97>] SyS_lsetxattr+0x87/0xe0 [ 28.064623] [<ffffffff81806f69>] system_call_fastpath+0x16/0x1b [ 28.064623] Code: 89 f8 48 89 e5 f6 82 40 3e a6 81 20 74 15 0f 1f 44 00 00 48 83 c0 01 0f b6 10 f6 82 40 3e a6 81 20 75 f0 5d c3 66 0f 1f 44 00 00 <80> 3f 00 55 48 89 e5 74 15 48 89 f8 0f 1f 40 00 48 83 c0 01 80 [ 28.064623] RIP [<ffffffff812ea780>] strlen+0x0/0x30 [ 28.064623] RSP <ffff8800297d3ca8> [ 28.064623] CR2: 0000000000000000 [ 28.093561] ---[ end trace de1055429a98a5be ]--- --- security/smack/smack_lsm.c | 2 +- security/smack/smackfs.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)security/smack/smack_lsm.c +1
-1@@ -923,7 +923,7 @@ static int smack_inode_setxattr(struct dentry *dentry, const char *name, rc = -EPERM ;if (rc ==0 && check_import) { - skp =smk_import_entry (value, size); + skp = size ?smk_import_entry (value, size) :NULL ;if (skp ==NULL || (check_star && (skp == &smack_known_star || skp == &smack_known_web))) rc = -EINVAL ;security/smack/smackfs.c +2
-3@@ -1677,7 +1677,7 @@ static ssize_t smk_write_onlycap(struct file *file, const char __user *buf,if (smack_onlycap !=NULL && smack_onlycap != skp)return -EPERM ; - data =kzalloc (count,GFP_KERNEL ); + data =kzalloc (count +1 ,GFP_KERNEL );if (data ==NULL )return -ENOMEM ;@@ -2228,7 +2228,7 @@ static ssize_t smk_write_syslog(struct file *file, const char __user *buf,if (!smack_privileged (CAP_MAC_ADMIN ))return -EPERM ; - data =kzalloc (count,GFP_KERNEL ); + data =kzalloc (count +1 ,GFP_KERNEL );if (data ==NULL )return -ENOMEM ; --To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
PATCH 3/3 Smack: remove unneeded NULL-termination from securtity label
In reply to Konstantin KhlebnikovValues of extended attributes are stored as binary blobs. NULL-termination of them isn't required. It just wastes disk space and confuses command-line tools like getfattr because they have to print that zero byte at the end.
This patch removes terminating zero byte from initial security label in smack_inode_init_security and cuts it out in function smack_inode_getsecurity which is used by syscall getxattr. This change seems completely safe, because function smk_parse_smack ignores everything after first zero byte.
--- security/smack/smack_lsm.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
security/smack/smack_lsm.c +3 -3
@@ -672,7 +672,7 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir, }if (len) - *len =strlen (isp) +1 ; + *len =strlen (isp);return 0 ; }
@@ -1076,7 +1076,7 @@ static int smack_inode_getsecurity(const struct inode *inode,if (strcmp (name,XATTR_SMACK_SUFFIX ) ==0 ) { isp =smk_of_inode (inode); - ilen =strlen (isp) +1 ; + ilen =strlen (isp); *buffer = isp;return ilen; }
@@ -1101,7 +1101,7 @@ static int smack_inode_getsecurity(const struct inode *inode,else return -EOPNOTSUPP ; - ilen =strlen (isp) +1 ; + ilen =strlen (isp);if (rc ==0 ) { *buffer = isp; rc = ilen;
Re: [PATCH 3/3] Smack: remove unneeded NULL-termination from securtity label
Quoted 10 lines · Konstantin Khlebnikov
Values of extended attributes are stored as binary blobs. NULL-termination of them isn't required. It just wastes disk space and confuses command-line tools like getfattr because they have to print that zero byte at the end.
This patch removes terminating zero byte from initial security label in
smack_inode_init_securityand cuts it out in functionsmack_inode_getsecuritywhich is used by syscall getxattr. This change seems completely safe, because functionsmk_parse_smackignores everything after first zero byte.Signed-off-by Konstantin Khlebnikov <[email protected]>
selinux doesn't seem to do it this way, and apparmor doesn't use these at all, but your reasoning does seem correct.
Quoted 35 lines
--- security/smack/smack_lsm.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)security/smack/smack_lsm.c +3
-3@@ -672,7 +672,7 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir, }if (len) - *len =strlen (isp) +1 ; + *len =strlen (isp);return 0 ; }@@ -1076,7 +1076,7 @@ static int smack_inode_getsecurity(const struct inode *inode,if (strcmp (name,XATTR_SMACK_SUFFIX ) ==0 ) { isp =smk_of_inode (inode); - ilen =strlen (isp) +1 ; + ilen =strlen (isp); *buffer = isp;return ilen; }@@ -1101,7 +1101,7 @@ static int smack_inode_getsecurity(const struct inode *inode,else return -EOPNOTSUPP ; - ilen =strlen (isp) +1 ; + ilen =strlen (isp);if (rc ==0 ) { *buffer = isp; rc = ilen;
Re: [PATCH 3/3] Smack: remove unneeded NULL-termination from securtity label
In reply to Konstantin KhlebnikovQuoted 10 lines · Konstantin Khlebnikov
Values of extended attributes are stored as binary blobs. NULL-termination of them isn't required. It just wastes disk space and confuses command-line tools like getfattr because they have to print that zero byte at the end.
This patch removes terminating zero byte from initial security label in
smack_inode_init_securityand cuts it out in functionsmack_inode_getsecuritywhich is used by syscall getxattr. This change seems completely safe, because functionsmk_parse_smackignores everything after first zero byte.Signed-off-by Konstantin Khlebnikov <[email protected]>
Applied to git://git.gitorious.org/smack-next/kernel.git#smack-for-3.18
Quoted 37 lines
--- security/smack/smack_lsm.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)security/smack/smack_lsm.c +3
-3@@ -672,7 +672,7 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir, }if (len) - *len =strlen (isp) +1 ; + *len =strlen (isp);return 0 ; }@@ -1076,7 +1076,7 @@ static int smack_inode_getsecurity(const struct inode *inode,if (strcmp (name,XATTR_SMACK_SUFFIX ) ==0 ) { isp =smk_of_inode (inode); - ilen =strlen (isp) +1 ; + ilen =strlen (isp); *buffer = isp;return ilen; }@@ -1101,7 +1101,7 @@ static int smack_inode_getsecurity(const struct inode *inode,else return -EOPNOTSUPP ; - ilen =strlen (isp) +1 ; + ilen =strlen (isp);if (rc ==0 ) { *buffer = isp; rc = ilen;
Quoted 11 lines · Konstantin Khlebnikov
Security operation ->inode_listsecurity is used for generating list of available extended attributes for syscall listxattr. Currently it's used only in nfs4 or if filesystem doesn't provide
i_op->listxattr.The list is the set of NULL-terminated names, one after the other. This method must include zero byte at the and into result.
Also this function must return length even if string does not fit into output buffer or it is NULL, see similar method in selinux and man listxattr.
Signed-off-by Konstantin Khlebnikov <[email protected]>
Quoted 26 lines
--- security/smack/smack_lsm.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)security/smack/smack_lsm.c +4
-5@@ -1122,13 +1122,12 @@ static int smack_inode_getsecurity(const struct inode *inode,static int smack_inode_listsecurity (struct inode *inode,char *buffer,size_t buffer_size) { -int len =strlen (XATTR_NAME_SMACK ); +int len =sizeof (XATTR_NAME_SMACK ); -if (buffer !=NULL && len <= buffer_size) { +if (buffer !=NULL && len <= buffer_size)memcpy (buffer,XATTR_NAME_SMACK , len); -return len; - } -return -EINVAL ; + +return len; }/**
Casey Schaufler
maintainerQuoted 11 lines · Konstantin Khlebnikov
Security operation ->inode_listsecurity is used for generating list of available extended attributes for syscall listxattr. Currently it's used only in nfs4 or if filesystem doesn't provide
i_op->listxattr.The list is the set of NULL-terminated names, one after the other. This method must include zero byte at the and into result.
Also this function must return length even if string does not fit into output buffer or it is NULL, see similar method in selinux and man listxattr.
Signed-off-by Konstantin Khlebnikov <[email protected]>
I plan to ack this patch set once I've run them through my tests.
Quoted 28 lines
--- security/smack/smack_lsm.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)security/smack/smack_lsm.c +4
-5@@ -1122,13 +1122,12 @@ static int smack_inode_getsecurity(const struct inode *inode,static int smack_inode_listsecurity (struct inode *inode,char *buffer,size_t buffer_size) { -int len =strlen (XATTR_NAME_SMACK ); +int len =sizeof (XATTR_NAME_SMACK ); -if (buffer !=NULL && len <= buffer_size) { +if (buffer !=NULL && len <= buffer_size)memcpy (buffer,XATTR_NAME_SMACK , len); -return len; - } -return -EINVAL ; + +return len; }/**
Quoted 11 lines · Konstantin Khlebnikov
Security operation ->inode_listsecurity is used for generating list of available extended attributes for syscall listxattr. Currently it's used only in nfs4 or if filesystem doesn't provide
i_op->listxattr.The list is the set of NULL-terminated names, one after the other. This method must include zero byte at the and into result.
Also this function must return length even if string does not fit into output buffer or it is NULL, see similar method in selinux and man listxattr.
Signed-off-by Konstantin Khlebnikov <[email protected]>
Applied to git://git.gitorious.org/smack-next/kernel.git#smack-for-3.18
Quoted 28 lines
--- security/smack/smack_lsm.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)security/smack/smack_lsm.c +4
-5@@ -1122,13 +1122,12 @@ static int smack_inode_getsecurity(const struct inode *inode,static int smack_inode_listsecurity (struct inode *inode,char *buffer,size_t buffer_size) { -int len =strlen (XATTR_NAME_SMACK ); +int len =sizeof (XATTR_NAME_SMACK ); -if (buffer !=NULL && len <= buffer_size) { +if (buffer !=NULL && len <= buffer_size)memcpy (buffer,XATTR_NAME_SMACK , len); -return len; - } -return -EINVAL ; + +return len; }/**