[PATCH] Fix deep stack usage in ncpfs

From: Petr Vandrovec <vandrove@vc.cvut.cz>


Arjan van de Ven pointed out to me there are no checks on name component
lengths in ncpfs, so potentially 4KB regions could be allocated on stack,
leading to the user controlled stack overflow.

It was using variable-sized arrays, so this snuck past the static
stack-usage checking tools.

As NCP is limited to 255 bytes on components, we can simple limit these
local variables to 256 bytes, and after this stack usage looks more
acceptable.  Length checking occurs inside ncp_vol2io, during
iocharset->codepage conversion.

As a side effect support for multibyte codepages now works as it should,
instead of returning -EINVAL whenever filename in 'codepage' encoding was
longer than in 'iocharset'.

Other part fixes typo where atime change updated ctime and not atime field.

fs/ncpfs/dir.c

@@ -270,8 +270,8 @@ __ncp_lookup_validate(struct dentry * dentry, struct nameidata *nd)
 	struct dentry *parent;
 	struct inode *dir;
 	struct ncp_entry_info finfo;
-	int res, val = 0, len = dentry->d_name.len + 1;
-	__u8 __name[len];
+	int res, val = 0, len;
+	__u8 __name[NCP_MAXPATHLEN + 1];
 
 	parent = dget_parent(dentry);
 	dir = parent->d_inode;
@@ -298,14 +298,15 @@ __ncp_lookup_validate(struct dentry * dentry, struct nameidata *nd)
 		dentry->d_parent->d_name.name, dentry->d_name.name,
 		NCP_GET_AGE(dentry));
 
+	len = sizeof(__name);
 	if (ncp_is_server_root(dir)) {
 		res = ncp_io2vol(server, __name, &len, dentry->d_name.name,
-						len-1, 1);
+				 dentry->d_name.len, 1);
 		if (!res)
 			res = ncp_lookup_volume(server, __name, &(finfo.i));
 	} else {
 		res = ncp_io2vol(server, __name, &len, dentry->d_name.name,
-						len-1, !ncp_preserve_case(dir));
+				 dentry->d_name.len, !ncp_preserve_case(dir));
 		if (!res)
 			res = ncp_obtain_info(server, dir, __name, &(finfo.i));
 	}
@@ -559,9 +560,9 @@ ncp_fill_cache(struct file *filp, void *dirent, filldir_t filldir,
 	int valid = 0;
 	int hashed = 0;
 	ino_t ino = 0;
-	__u8 __name[256];
+	__u8 __name[NCP_MAXPATHLEN + 1];
 
-	qname.len = 256;
+	qname.len = sizeof(__name);
 	if (ncp_vol2io(NCP_SERVER(inode), __name, &qname.len,
 			entry->i.entryName, entry->i.nameLen,
 			!ncp_preserve_entry_case(inode, entry->i.NSCreator)))
@@ -762,16 +763,19 @@ int ncp_conn_logged_in(struct super_block *sb)
 {
 	struct ncp_server* server = NCP_SBP(sb);
 	struct nw_info_struct i;
-	int result, len = strlen(server->m.mounted_vol) + 1;
-	__u8 __name[len];
+	int result;
 
 	if (ncp_single_volume(server)) {
+		int len;
 		struct dentry* dent;
+		__u8 __name[NCP_MAXPATHLEN + 1];
 
-		result = -ENOENT;
-		if (ncp_io2vol(server, __name, &len, server->m.mounted_vol,
-								len-1, 1))
+		len = sizeof(__name);
+		result = ncp_io2vol(server, __name, &len, server->m.mounted_vol,
+				    strlen(server->m.mounted_vol), 1);
+		if (result)
 			goto out;
+		result = -ENOENT;
 		if (ncp_lookup_volume(server, __name, &i)) {
 			PPRINTK("ncp_conn_logged_in: %s not found\n",
 				server->m.mounted_vol);
@@ -802,8 +806,8 @@ static struct dentry *ncp_lookup(struct inode *dir, struct dentry *dentry, struc
 	struct ncp_server *server = NCP_SERVER(dir);
 	struct inode *inode = NULL;
 	struct ncp_entry_info finfo;
-	int error, res, len = dentry->d_name.len + 1;
-	__u8 __name[len];
+	int error, res, len;
+	__u8 __name[NCP_MAXPATHLEN + 1];
 
 	lock_kernel();
 	error = -EIO;
@@ -813,14 +817,15 @@ static struct dentry *ncp_lookup(struct inode *dir, struct dentry *dentry, struc
 	PPRINTK("ncp_lookup: server lookup for %s/%s\n",
 		dentry->d_parent->d_name.name, dentry->d_name.name);
 
+	len = sizeof(__name);
 	if (ncp_is_server_root(dir)) {
 		res = ncp_io2vol(server, __name, &len, dentry->d_name.name,
-						len-1, 1);
+				 dentry->d_name.len, 1);
 		if (!res)
 			res = ncp_lookup_volume(server, __name, &(finfo.i));
 	} else {
 		res = ncp_io2vol(server, __name, &len, dentry->d_name.name,
-						len-1, !ncp_preserve_case(dir));
+				 dentry->d_name.len, !ncp_preserve_case(dir));
 		if (!res)
 			res = ncp_obtain_info(server, dir, __name, &(finfo.i));
 	}
@@ -885,20 +890,22 @@ int ncp_create_new(struct inode *dir, struct dentry *dentry, int mode,
 {
 	struct ncp_server *server = NCP_SERVER(dir);
 	struct ncp_entry_info finfo;
-	int error, result, len = dentry->d_name.len + 1;
+	int error, result, len;
 	int opmode;
-	__u8 __name[len];
+	__u8 __name[NCP_MAXPATHLEN + 1];
 	
 	PPRINTK("ncp_create_new: creating %s/%s, mode=%x\n",
 		dentry->d_parent->d_name.name, dentry->d_name.name, mode);
+
 	error = -EIO;
 	lock_kernel();
 	if (!ncp_conn_valid(server))
 		goto out;
 
 	ncp_age_dentry(server, dentry);
+	len = sizeof(__name);
 	error = ncp_io2vol(server, __name, &len, dentry->d_name.name,
-						len-1, !ncp_preserve_case(dir));
+			   dentry->d_name.len, !ncp_preserve_case(dir));
 	if (error)
 		goto out;
 
@@ -952,19 +959,21 @@ static int ncp_mkdir(struct inode *dir, struct dentry *dentry, int mode)
 {
 	struct ncp_entry_info finfo;
 	struct ncp_server *server = NCP_SERVER(dir);
-	int error, len = dentry->d_name.len + 1;
-	__u8 __name[len];
+	int error, len;
+	__u8 __name[NCP_MAXPATHLEN + 1];
 
 	DPRINTK("ncp_mkdir: making %s/%s\n",
 		dentry->d_parent->d_name.name, dentry->d_name.name);
+
 	error = -EIO;
 	lock_kernel();
 	if (!ncp_conn_valid(server))
 		goto out;
 
 	ncp_age_dentry(server, dentry);
+	len = sizeof(__name);
 	error = ncp_io2vol(server, __name, &len, dentry->d_name.name,
-						len-1, !ncp_preserve_case(dir));
+			   dentry->d_name.len, !ncp_preserve_case(dir));
 	if (error)
 		goto out;
 
@@ -992,8 +1001,8 @@ static int ncp_mkdir(struct inode *dir, struct dentry *dentry, int mode)
 static int ncp_rmdir(struct inode *dir, struct dentry *dentry)
 {
 	struct ncp_server *server = NCP_SERVER(dir);
-	int error, result, len = dentry->d_name.len + 1;
-	__u8 __name[len];
+	int error, result, len;
+	__u8 __name[NCP_MAXPATHLEN + 1];
 
 	DPRINTK("ncp_rmdir: removing %s/%s\n",
 		dentry->d_parent->d_name.name, dentry->d_name.name);
@@ -1007,8 +1016,9 @@ static int ncp_rmdir(struct inode *dir, struct dentry *dentry)
 	if (!d_unhashed(dentry))
 		goto out;
 
+	len = sizeof(__name);
 	error = ncp_io2vol(server, __name, &len, dentry->d_name.name,
-						len-1, !ncp_preserve_case(dir));
+			   dentry->d_name.len, !ncp_preserve_case(dir));
 	if (error)
 		goto out;
 
@@ -1110,9 +1120,8 @@ static int ncp_rename(struct inode *old_dir, struct dentry *old_dentry,
 {
 	struct ncp_server *server = NCP_SERVER(old_dir);
 	int error;
-	int old_len = old_dentry->d_name.len + 1;
-	int new_len = new_dentry->d_name.len + 1;
-	__u8 __old_name[old_len], __new_name[new_len];
+	int old_len, new_len;
+	__u8 __old_name[NCP_MAXPATHLEN + 1], __new_name[NCP_MAXPATHLEN + 1];
 
 	DPRINTK("ncp_rename: %s/%s to %s/%s\n",
 		old_dentry->d_parent->d_name.name, old_dentry->d_name.name,
@@ -1126,15 +1135,17 @@ static int ncp_rename(struct inode *old_dir, struct dentry *old_dentry,
 	ncp_age_dentry(server, old_dentry);
 	ncp_age_dentry(server, new_dentry);
 
+	old_len = sizeof(__old_name);
 	error = ncp_io2vol(server, __old_name, &old_len,
-					old_dentry->d_name.name, old_len-1,
-					!ncp_preserve_case(old_dir));
+			   old_dentry->d_name.name, old_dentry->d_name.len,
+			   !ncp_preserve_case(old_dir));
 	if (error)
 		goto out;
 
+	new_len = sizeof(__new_name);
 	error = ncp_io2vol(server, __new_name, &new_len,
-					new_dentry->d_name.name, new_len-1,
-					!ncp_preserve_case(new_dir));
+			   new_dentry->d_name.name, new_dentry->d_name.len,
+			   !ncp_preserve_case(new_dir));
 	if (error)
 		goto out;
 

fs/ncpfs/inode.c

@@ -917,7 +917,7 @@ int ncp_notify_change(struct dentry *dentry, struct iattr *attr)
 	if ((attr->ia_valid & ATTR_ATIME) != 0) {
 		__u16 dummy;
 		info_mask |= (DM_LAST_ACCESS_DATE);
-		ncp_date_unix2dos(attr->ia_ctime.tv_sec,
+		ncp_date_unix2dos(attr->ia_atime.tv_sec,
 				  &(dummy), &(info.lastAccessDate));
 		info.lastAccessDate = le16_to_cpu(info.lastAccessDate);
 	}