[Kernel-janitors] [patch 2.6.1] audit *_user, get rid of verify_area

Randy.Dunlap rddunlap at osdl.org
Fri Jan 16 14:10:59 PST 2004


On Fri, 16 Jan 2004 22:45:32 +0100 Domen Puncer <domen at coderock.org> wrote:

| Hi.
| 
| To be aplied on top of CONFIG_LEDMAN patch, or you'll get -11 lines offsets.
| 
| I left one copy_*_user, because locking needs to be fixed around it too.
| About that s/break/return/ in switch: at end of switch there is a "return 0".
| Didn't compile test it, since it's a motorola driver.
| 
| 
| 	Domen
| 
| --- c/drivers/serial/mcfserial.c	2004-01-16 22:28:31.000000000 +0100
| +++ a/drivers/serial/mcfserial.c	2004-01-16 22:40:44.000000000 +0100
|  		case TIOCSERGSTRUCT:
| -			error = verify_area(VERIFY_WRITE, (void *) arg,
| -						sizeof(struct mcf_serial));
| -			if (error)
| -				return error;
| -			copy_to_user((struct mcf_serial *) arg,
| -				    info, sizeof(struct mcf_serial));
| -			return 0;
| +			if (copy_to_user((struct mcf_serial *) arg,
| +				    info, sizeof(struct mcf_serial)))
| +			return -EFAULT;

what if copy_to_user() above returns 0?
is it supposed to fall through?

rest of it looks fine.
Please fix & resend (unless it's correct as is)

|  		case TIOCMGET:
| -			if ((error = verify_area(VERIFY_WRITE, (void *) arg,
| -                            sizeof(unsigned int))))
| -                                return(error);
|  			val = mcfrs_getsignals(info);
| -			put_user(val, (unsigned int *) arg);
| -			break;
| +			return put_user(val, (unsigned int *) arg);
|  


--
~Randy
Everything is relative.



More information about the Kernel-janitors mailing list