[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-security-module
Subject:    Re: how to set the xattr information of current task
From:       Casey Schaufler <casey () schaufler-ca ! com>
Date:       2007-02-07 2:50:18
Message-ID: 20070207025018.70775.qmail () web36601 ! mail ! mud ! yahoo ! com
[Download RAW message or body]


--- Ian jonhson <jonhson.ian@gmail.com> wrote:


> static int my_setprocattr(struct task_struct *p,
> char *name, void
> *value, size_t size)
> {
> 	my_struct sl;
> 	my_struct* ts;
> 	
>   	if(current != p)
>     		return -EACCES;
> 
>   	if(!size)
>     		return -ERANGE;
> 
>   	if(!strcmp(name, "current"))
> 	{
> 		if (copy_from_user(&sl, value, sizeof(my_struct)))
> {
> 			return -EFAULT;
> 		}


"value" is a kernel address, not a user address.
You can replace this with:

                memcpy(&sl, value, sizeof(my_struct));

> 		
> 		ts = p->security;
> 		if(ts)
> 		{
> 			ts->v1 = sl.v1;
> 			ts->v2 = sl.v2;
> 
> 		}
> 		else
> 		{
> 			ts->v1 = UNDEFINE_V1;
> 			ts->v2 = UNDEFINE_V2;
> 		}

If ts is NULL this will not work, you know.

> 		
> 	
> 		return 0;				
> 	}		
> 
> 	return size;
> }
> 
> After compiling the module and insmod it, I run my
> setselfattr codes
> mentioned last mail and got another error message:
> 
> write failed due to: Bad address
> 
> Maybe, there are indeed something wrong with my
> hook.

The change from copy_from_user to memcpy will
take care of today's problem, but you still need to
be carefull about your pointers. If ts is NULL
the code presented will crash.

Also, it is very dangerous to assume that the
value passed down is big enough. Check that
"size" is correct. I encourage you to validate
the data passed to ensure that it is reasonable.

Oh, and as Seth pointed out, follow the kernel
coding style. There are reasons that sytle has
been adopted, and some of those reasons have
merit.

Good luck, and let us know what happens next.
 

Casey Schaufler
casey@schaufler-ca.com
-
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
[prev in list] [next in list] [prev in thread] [next in thread]