ncpfs: don't allow negative timeouts
PATCH 1/1 ncpfs: don't allow negative timeouts
This code causes a static checker warning because it's a user controlled variable where we cap the upper bound but not the lower bound. If someone passes us a negative timeout then I guess lets set it to the default.
fs/ncpfs/ioctl.c +1 -1
@@ -525,7 +525,7 @@ static long __ncp_ioctl(struct inode *inode, unsigned int cmd, unsigned long argswitch (rqdata.cmd) {case NCP_LOCK_EX :case NCP_LOCK_SH : -if (rqdata.timeout =0 ) +if (rqdata.timeout <=0 ) rqdata.timeout =NCP_LOCK_DEFAULT_TIMEOUT ;else if (rqdata.timeout >NCP_LOCK_MAX_TIMEOUT ) rqdata.timeout =NCP_LOCK_MAX_TIMEOUT ;
Quoted 6 lines · Dan Carpenter
This code causes a static checker warning because it's a user controlled variable where we cap the upper bound but not the lower bound. If someone passes us a negative timeout then I guess lets set it to the default.
Signed-off-by Dan Carpenter <[email protected]>
IMHO it would be better to return error from the ioctl. Currently it would just wrap when converting to u16 anyway...
Honza
Quoted 14 lines
fs/ncpfs/ioctl.c +1
-1@@ -525,7 +525,7 @@ static long __ncp_ioctl(struct inode *inode, unsigned int cmd, unsigned long argswitch (rqdata.cmd) {case NCP_LOCK_EX :case NCP_LOCK_SH : -if (rqdata.timeout =0 ) +if (rqdata.timeout <=0 ) rqdata.timeout =NCP_LOCK_DEFAULT_TIMEOUT ;else if (rqdata.timeout >NCP_LOCK_MAX_TIMEOUT ) rqdata.timeout =NCP_LOCK_MAX_TIMEOUT ;
Signature · 2 lines
Jan Kara <[email protected]> SUSE Labs, CR
PATCH v2 1/1 ncpfs: don't allow negative timeouts
This code causes a static checker warning because it's a user controlled variable where we cap the upper bound but not the lower bound. Let's return an -EINVAL for negative timeouts.
---
v2: in the original I just ignored the invalid data and went with the
default but now it returns -EINVAL.
fs/ncpfs/ioctl.c +3 -1
@@ -525,7 +525,9 @@ static long __ncp_ioctl(struct inode *inode, unsigned int cmd, unsigned long argswitch (rqdata.cmd) {case NCP_LOCK_EX :case NCP_LOCK_SH : -if (rqdata.timeout =0 ) +if (rqdata.timeout <0 ) +return -EINVAL ; +else if (rqdata.timeout =0 ) rqdata.timeout =NCP_LOCK_DEFAULT_TIMEOUT ;else if (rqdata.timeout >NCP_LOCK_MAX_TIMEOUT ) rqdata.timeout =NCP_LOCK_MAX_TIMEOUT ;
Quoted 5 lines · Dan Carpenter
This code causes a static checker warning because it's a user controlled variable where we cap the upper bound but not the lower bound. Let's return an
-EINVALfor negative timeouts.Signed-off-by Dan Carpenter <[email protected]>
Looks good. You can add:
Honza
Quoted 19 lines
---
v2: in the original I just ignored the invalid data and went with the
default but now it returns-EINVAL.fs/ncpfs/ioctl.c +3
-1@@ -525,7 +525,9 @@ static long __ncp_ioctl(struct inode *inode, unsigned int cmd, unsigned long argswitch (rqdata.cmd) {case NCP_LOCK_EX :case NCP_LOCK_SH : -if (rqdata.timeout =0 ) +if (rqdata.timeout <0 ) +return -EINVAL ; +else if (rqdata.timeout =0 ) rqdata.timeout =NCP_LOCK_DEFAULT_TIMEOUT ;else if (rqdata.timeout >NCP_LOCK_MAX_TIMEOUT ) rqdata.timeout =NCP_LOCK_MAX_TIMEOUT ;
Signature · 2 lines
Jan Kara <[email protected]> SUSE Labs, CR