2010-02-08 11:41:20

by wzt wzt

Subject: [PATCH] security/smack/smack.h: Fix smk_known length

in security/smack/smack.h:
struct smack_known {
struct list_head list;
char smk_known[SMK_LABELLEN];
but in security/smack/smack_access.c
void smack_from_cipso(u32 level, char *cp, char *result)
{
strncpy(result, final, SMK_MAXLEN);
}
miss '\0'.

Signed-off-by: wzt <[email protected]>
Cc: Jmorris <[email protected]>

diff --git a/security/smack/smack.h b/security/smack/smack.h
index c6e9aca..600474b 100644
--- a/security/smack/smack.h
+++ b/security/smack/smack.h
@@ -109,7 +109,7 @@ struct smk_netlbladdr {
*/
struct smack_known {
struct list_head list;
- char smk_known[SMK_LABELLEN];
+ char smk_known[SMK_LABELLEN + 1]; /*
smk_known + \0 */
u32 smk_secid;
struct smack_cipso *smk_cipso;
spinlock_t smk_cipsolock; /* for changing cipso map */

2010-02-08 21:28:38

by James Morris

Subject: Re: [PATCH] security/smack/smack.h: Fix smk_known length

(note: please copy security/ patches to the LSM list, cc'd, along with the
Smack maintainer).


On Mon, 8 Feb 2010, wzt wzt wrote:

> in security/smack/smack.h:
> struct smack_known {
> struct list_head list;
> char smk_known[SMK_LABELLEN];
> but in security/smack/smack_access.c
> void smack_from_cipso(u32 level, char *cp, char *result)
> {
> strncpy(result, final, SMK_MAXLEN);
> }
> miss '\0'.
>
> Signed-off-by: wzt <[email protected]>
> Cc: Jmorris <[email protected]>
>
> diff --git a/security/smack/smack.h b/security/smack/smack.h
> index c6e9aca..600474b 100644
> --- a/security/smack/smack.h
> +++ b/security/smack/smack.h
> @@ -109,7 +109,7 @@ struct smk_netlbladdr {
> */
> struct smack_known {
> struct list_head list;
> - char smk_known[SMK_LABELLEN];
> + char smk_known[SMK_LABELLEN + 1]; /*
> smk_known + \0 */
> u32 smk_secid;
> struct smack_cipso *smk_cipso;
> spinlock_t smk_cipsolock; /* for changing cipso map */
>

--
James Morris
<[email protected]>

2010-02-09 03:14:29

by Casey Schaufler

Subject: Re: [PATCH] security/smack/smack.h: Fix smk_known length

James Morris wrote:
> (note: please copy security/ patches to the LSM list, cc'd, along with the
> Smack maintainer).
>
>
> On Mon, 8 Feb 2010, wzt wzt wrote:
>
>
>> in security/smack/smack.h:
>> struct smack_known {
>> struct list_head list;
>> char smk_known[SMK_LABELLEN];
>> but in security/smack/smack_access.c
>> void smack_from_cipso(u32 level, char *cp, char *result)
>> {
>> strncpy(result, final, SMK_MAXLEN);
>> }
>> miss '\0'.
>>
>> Signed-off-by: wzt <[email protected]>
>> Cc: Jmorris <[email protected]>
>>
>> diff --git a/security/smack/smack.h b/security/smack/smack.h
>> index c6e9aca..600474b 100644
>> --- a/security/smack/smack.h
>> +++ b/security/smack/smack.h
>> @@ -109,7 +109,7 @@ struct smk_netlbladdr {
>> */
>> struct smack_known {
>> struct list_head list;
>> - char smk_known[SMK_LABELLEN];
>> + char smk_known[SMK_LABELLEN + 1]; /*
>> smk_known + \0 */
>>

SMK_LABELLEN is already SMK_MAXLEN+1. That's why the strncpy() above
uses SMK_MAXLEN, so that maximum copied will be SMK_MAXLEN+1, including
the trailing '\0'. The only possible case it could miss would be a
label that is SMK_LABELLEN (24 bytes) long, and that would be an error
because Smack labels are limited to SMK_MAXLEN (23 bytes) plus the
trailing '\0'. The strncpy() could easily be strcpy(), as only labels
that have been imported or hand crafted in the code will be in "from".


>> u32 smk_secid;
>> struct smack_cipso *smk_cipso;
>> spinlock_t smk_cipsolock; /* for changing cipso map */
>>
>>