From: Petr Vandrovec 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. --- 25-akpm/fs/ncpfs/dir.c | 73 +++++++++++++++++++++++++++-------------------- 25-akpm/fs/ncpfs/inode.c | 2 - 2 files changed, 43 insertions(+), 32 deletions(-) diff -puN fs/ncpfs/dir.c~ncpfs-stack-usage-fix fs/ncpfs/dir.c --- 25/fs/ncpfs/dir.c~ncpfs-stack-usage-fix Thu Jan 22 15:59:31 2004 +++ 25-akpm/fs/ncpfs/dir.c Thu Jan 22 15:59:31 2004 @@ -270,8 +270,8 @@ __ncp_lookup_validate(struct dentry * de 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 * de 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 * 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_bloc { 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 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 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, st { 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 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 @@ out: 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, 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_ { 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_ 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; diff -puN fs/ncpfs/inode.c~ncpfs-stack-usage-fix fs/ncpfs/inode.c --- 25/fs/ncpfs/inode.c~ncpfs-stack-usage-fix Thu Jan 22 15:59:31 2004 +++ 25-akpm/fs/ncpfs/inode.c Thu Jan 22 15:59:31 2004 @@ -917,7 +917,7 @@ int ncp_notify_change(struct dentry *den 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); } _