* [PATCH] smack:beyond ARRAY_SIZE of data
@ 2009-05-20 15:19 Roel Kluin
2009-05-20 22:27 ` Casey Schaufler
0 siblings, 1 reply; 6+ messages in thread
From: Roel Kluin @ 2009-05-20 15:19 UTC (permalink / raw)
To: jmorris; +Cc: lkml, linux-security-module, Andrew Morton
Do not go beyond ARRAY_SIZE of data
Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
index e03a7e1..7407e5c 100644
--- a/security/smack/smackfs.c
+++ b/security/smack/smackfs.c
@@ -797,7 +797,7 @@ static ssize_t smk_write_netlbladdr(struct file *file, const char __user *buf,
return -EPERM;
if (*ppos != 0)
return -EINVAL;
- if (count < SMK_NETLBLADDRMIN || count > SMK_NETLBLADDRMAX)
+ if (count < SMK_NETLBLADDRMIN || count >= SMK_NETLBLADDRMAX)
return -EINVAL;
if (copy_from_user(data, buf, count) != 0)
return -EFAULT;
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] smack:beyond ARRAY_SIZE of data
2009-05-20 15:19 [PATCH] smack:beyond ARRAY_SIZE of data Roel Kluin
@ 2009-05-20 22:27 ` Casey Schaufler
2009-05-21 16:42 ` Roel Kluin
0 siblings, 1 reply; 6+ messages in thread
From: Casey Schaufler @ 2009-05-20 22:27 UTC (permalink / raw)
To: Roel Kluin; +Cc: jmorris, lkml, linux-security-module, Andrew Morton
Roel Kluin wrote:
> Do not go beyond ARRAY_SIZE of data
>
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> ---
> diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
> index e03a7e1..7407e5c 100644
> --- a/security/smack/smackfs.c
> +++ b/security/smack/smackfs.c
> @@ -797,7 +797,7 @@ static ssize_t smk_write_netlbladdr(struct file *file, const char __user *buf,
> return -EPERM;
> if (*ppos != 0)
> return -EINVAL;
> - if (count < SMK_NETLBLADDRMIN || count > SMK_NETLBLADDRMAX)
> + if (count < SMK_NETLBLADDRMIN || count >= SMK_NETLBLADDRMAX)
>
There is a problem here, but this won't fix it. The buffer needs to be
allocated bigger than the potential contents (should be
SMK_NETLBLADDRMAX + 1 instead of SMK_NETLBLADDRMAX. Your patch will clip
the last byte off of a maximum length specification.
> return -EINVAL;
> if (copy_from_user(data, buf, count) != 0)
> return -EFAULT;
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] smack:beyond ARRAY_SIZE of data
2009-05-20 22:27 ` Casey Schaufler
@ 2009-05-21 16:42 ` Roel Kluin
2009-05-22 2:30 ` Casey Schaufler
0 siblings, 1 reply; 6+ messages in thread
From: Roel Kluin @ 2009-05-21 16:42 UTC (permalink / raw)
To: Casey Schaufler; +Cc: jmorris, lkml, linux-security-module, Andrew Morton
Do not go beyond ARRAY_SIZE of data
Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
>> - if (count < SMK_NETLBLADDRMIN || count > SMK_NETLBLADDRMAX)
>> + if (count < SMK_NETLBLADDRMIN || count >= SMK_NETLBLADDRMAX)
>>
>
> There is a problem here, but this won't fix it. The buffer needs to be
> allocated bigger than the potential contents (should be
> SMK_NETLBLADDRMAX + 1 instead of SMK_NETLBLADDRMAX. Your patch will clip
> the last byte off of a maximum length specification.
Ok, how about this?
diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
index e03a7e1..10a4604 100644
--- a/security/smack/smackfs.c
+++ b/security/smack/smackfs.c
@@ -775,7 +775,7 @@ static ssize_t smk_write_netlbladdr(struct file *file, const char __user *buf,
struct sockaddr_in newname;
char smack[SMK_LABELLEN];
char *sp;
- char data[SMK_NETLBLADDRMAX];
+ char data[SMK_NETLBLADDRMAX + 1];
char *host = (char *)&newname.sin_addr.s_addr;
int rc;
struct netlbl_audit audit_info;
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] smack:beyond ARRAY_SIZE of data
2009-05-21 16:42 ` Roel Kluin
@ 2009-05-22 2:30 ` Casey Schaufler
2009-05-22 2:42 ` James Morris
0 siblings, 1 reply; 6+ messages in thread
From: Casey Schaufler @ 2009-05-22 2:30 UTC (permalink / raw)
To: Roel Kluin; +Cc: jmorris, lkml, linux-security-module, Andrew Morton
Roel Kluin wrote:
> Do not go beyond ARRAY_SIZE of data
>
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
>
Acked-by: Casey Schaufler <casey@schaufler-ca.com>
Thank you. The fix is appreciated.
> ---
>
>>> - if (count < SMK_NETLBLADDRMIN || count > SMK_NETLBLADDRMAX)
>>> + if (count < SMK_NETLBLADDRMIN || count >= SMK_NETLBLADDRMAX)
>>>
>>>
>> There is a problem here, but this won't fix it. The buffer needs to be
>> allocated bigger than the potential contents (should be
>> SMK_NETLBLADDRMAX + 1 instead of SMK_NETLBLADDRMAX. Your patch will clip
>> the last byte off of a maximum length specification.
>>
>
> Ok, how about this?
>
> diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
> index e03a7e1..10a4604 100644
> --- a/security/smack/smackfs.c
> +++ b/security/smack/smackfs.c
> @@ -775,7 +775,7 @@ static ssize_t smk_write_netlbladdr(struct file *file, const char __user *buf,
> struct sockaddr_in newname;
> char smack[SMK_LABELLEN];
> char *sp;
> - char data[SMK_NETLBLADDRMAX];
> + char data[SMK_NETLBLADDRMAX + 1];
> char *host = (char *)&newname.sin_addr.s_addr;
> int rc;
> struct netlbl_audit audit_info;
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] smack:beyond ARRAY_SIZE of data
2009-05-22 2:30 ` Casey Schaufler
@ 2009-05-22 2:42 ` James Morris
2009-05-22 4:35 ` Casey Schaufler
0 siblings, 1 reply; 6+ messages in thread
From: James Morris @ 2009-05-22 2:42 UTC (permalink / raw)
To: Casey Schaufler; +Cc: Roel Kluin, lkml, linux-security-module, Andrew Morton
On Thu, 21 May 2009, Casey Schaufler wrote:
> Roel Kluin wrote:
> > Do not go beyond ARRAY_SIZE of data
> >
> > Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> >
>
> Acked-by: Casey Schaufler <casey@schaufler-ca.com>
>
> Thank you. The fix is appreciated.
>
Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6#next
(This needs CAP_MAC_ADMIN to trigger, so I'll assume this is not
security-critical, but yell if you disagree).
- James
--
James Morris
<jmorris@namei.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] smack:beyond ARRAY_SIZE of data
2009-05-22 2:42 ` James Morris
@ 2009-05-22 4:35 ` Casey Schaufler
0 siblings, 0 replies; 6+ messages in thread
From: Casey Schaufler @ 2009-05-22 4:35 UTC (permalink / raw)
To: James Morris; +Cc: Roel Kluin, lkml, linux-security-module, Andrew Morton
James Morris wrote:
> On Thu, 21 May 2009, Casey Schaufler wrote:
>
>
>> Roel Kluin wrote:
>>
>>> Do not go beyond ARRAY_SIZE of data
>>>
>>> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
>>>
>>>
>> Acked-by: Casey Schaufler <casey@schaufler-ca.com>
>>
>> Thank you. The fix is appreciated.
>>
>>
>
> Applied to
> git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6#next
>
> (This needs CAP_MAC_ADMIN to trigger, so I'll assume this is not
> security-critical, but yell if you disagree).
>
>
Not security critical.
> - James
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-05-22 4:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-05-20 15:19 [PATCH] smack:beyond ARRAY_SIZE of data Roel Kluin
2009-05-20 22:27 ` Casey Schaufler
2009-05-21 16:42 ` Roel Kluin
2009-05-22 2:30 ` Casey Schaufler
2009-05-22 2:42 ` James Morris
2009-05-22 4:35 ` Casey Schaufler
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox