diff options
author | Steve French <sfrench@us.ibm.com> | 2008-12-17 20:41:20 -0500 |
---|---|---|
committer | Steve French <sfrench@us.ibm.com> | 2008-12-25 21:29:13 -0500 |
commit | c6fbba0546d3ead18d4a623e76e28bcbaa66a325 (patch) | |
tree | fa2789812091b4feb35bcb9bcae0979ef1d588c5 | |
parent | ac6a3ef405f314c206906463ca9913a826a577ee (diff) |
[CIFS] make sure that DFS pathnames are properly formed
The paths in a DFS request are supposed to only have a single preceding
backslash, but we are sending them with a double backslash. This is
exposing a bug in Windows where it also sends a path in the response
that has a double backslash.
The existing code that builds the mount option string however expects a
double backslash prefix in a couple of places when it tries to use the
path returned by build_path_from_dentry. Fix compose_mount_options to
expect properly formed DFS paths (single backslash at front).
Also clean up error handling in that function. There was a possible
NULL pointer dereference and situations where a partially built option
string would be returned.
Tested against Samba 3.0.28-ish server and Samba 3.3 and Win2k8.
CC: Stable <stable@kernel.org>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: Steve French <sfrench@us.ibm.com>
-rw-r--r-- | fs/cifs/CHANGES | 3 | ||||
-rw-r--r-- | fs/cifs/cifs_dfs_ref.c | 48 |
2 files changed, 38 insertions, 13 deletions
diff --git a/fs/cifs/CHANGES b/fs/cifs/CHANGES index 5ab6bcce880a..080703a15f44 100644 --- a/fs/cifs/CHANGES +++ b/fs/cifs/CHANGES | |||
@@ -4,7 +4,8 @@ Add "forcemandatorylock" mount option to allow user to use mandatory | |||
4 | rather than posix (advisory) byte range locks, even though server would | 4 | rather than posix (advisory) byte range locks, even though server would |
5 | support posix byte range locks. Fix query of root inode when prefixpath | 5 | support posix byte range locks. Fix query of root inode when prefixpath |
6 | specified and user does not have access to query information about the | 6 | specified and user does not have access to query information about the |
7 | top of the share. | 7 | top of the share. Fix problem in 2.6.28 resolving DFS paths to |
8 | Samba servers (worked to Windows). | ||
8 | 9 | ||
9 | Version 1.55 | 10 | Version 1.55 |
10 | ------------ | 11 | ------------ |
diff --git a/fs/cifs/cifs_dfs_ref.c b/fs/cifs/cifs_dfs_ref.c index e1c18362ba46..85c0a74d034d 100644 --- a/fs/cifs/cifs_dfs_ref.c +++ b/fs/cifs/cifs_dfs_ref.c | |||
@@ -122,7 +122,7 @@ static char *compose_mount_options(const char *sb_mountdata, | |||
122 | char **devname) | 122 | char **devname) |
123 | { | 123 | { |
124 | int rc; | 124 | int rc; |
125 | char *mountdata; | 125 | char *mountdata = NULL; |
126 | int md_len; | 126 | int md_len; |
127 | char *tkn_e; | 127 | char *tkn_e; |
128 | char *srvIP = NULL; | 128 | char *srvIP = NULL; |
@@ -136,10 +136,9 @@ static char *compose_mount_options(const char *sb_mountdata, | |||
136 | *devname = cifs_get_share_name(ref->node_name); | 136 | *devname = cifs_get_share_name(ref->node_name); |
137 | rc = dns_resolve_server_name_to_ip(*devname, &srvIP); | 137 | rc = dns_resolve_server_name_to_ip(*devname, &srvIP); |
138 | if (rc != 0) { | 138 | if (rc != 0) { |
139 | cERROR(1, ("%s: Failed to resolve server part of %s to IP", | 139 | cERROR(1, ("%s: Failed to resolve server part of %s to IP: %d", |
140 | __func__, *devname)); | 140 | __func__, *devname, rc));; |
141 | mountdata = ERR_PTR(rc); | 141 | goto compose_mount_options_err; |
142 | goto compose_mount_options_out; | ||
143 | } | 142 | } |
144 | /* md_len = strlen(...) + 12 for 'sep+prefixpath=' | 143 | /* md_len = strlen(...) + 12 for 'sep+prefixpath=' |
145 | * assuming that we have 'unc=' and 'ip=' in | 144 | * assuming that we have 'unc=' and 'ip=' in |
@@ -149,8 +148,8 @@ static char *compose_mount_options(const char *sb_mountdata, | |||
149 | strlen(ref->node_name) + 12; | 148 | strlen(ref->node_name) + 12; |
150 | mountdata = kzalloc(md_len+1, GFP_KERNEL); | 149 | mountdata = kzalloc(md_len+1, GFP_KERNEL); |
151 | if (mountdata == NULL) { | 150 | if (mountdata == NULL) { |
152 | mountdata = ERR_PTR(-ENOMEM); | 151 | rc = -ENOMEM; |
153 | goto compose_mount_options_out; | 152 | goto compose_mount_options_err; |
154 | } | 153 | } |
155 | 154 | ||
156 | /* copy all options except of unc,ip,prefixpath */ | 155 | /* copy all options except of unc,ip,prefixpath */ |
@@ -197,18 +196,32 @@ static char *compose_mount_options(const char *sb_mountdata, | |||
197 | 196 | ||
198 | /* find & copy prefixpath */ | 197 | /* find & copy prefixpath */ |
199 | tkn_e = strchr(ref->node_name + 2, '\\'); | 198 | tkn_e = strchr(ref->node_name + 2, '\\'); |
200 | if (tkn_e == NULL) /* invalid unc, missing share name*/ | 199 | if (tkn_e == NULL) { |
201 | goto compose_mount_options_out; | 200 | /* invalid unc, missing share name*/ |
201 | rc = -EINVAL; | ||
202 | goto compose_mount_options_err; | ||
203 | } | ||
202 | 204 | ||
205 | /* | ||
206 | * this function gives us a path with a double backslash prefix. We | ||
207 | * require a single backslash for DFS. Temporarily increment fullpath | ||
208 | * to put it in the proper form and decrement before freeing it. | ||
209 | */ | ||
203 | fullpath = build_path_from_dentry(dentry); | 210 | fullpath = build_path_from_dentry(dentry); |
211 | if (!fullpath) { | ||
212 | rc = -ENOMEM; | ||
213 | goto compose_mount_options_err; | ||
214 | } | ||
215 | ++fullpath; | ||
204 | tkn_e = strchr(tkn_e + 1, '\\'); | 216 | tkn_e = strchr(tkn_e + 1, '\\'); |
205 | if (tkn_e || strlen(fullpath) - (ref->path_consumed)) { | 217 | if (tkn_e || (strlen(fullpath) - ref->path_consumed)) { |
206 | strncat(mountdata, &sep, 1); | 218 | strncat(mountdata, &sep, 1); |
207 | strcat(mountdata, "prefixpath="); | 219 | strcat(mountdata, "prefixpath="); |
208 | if (tkn_e) | 220 | if (tkn_e) |
209 | strcat(mountdata, tkn_e + 1); | 221 | strcat(mountdata, tkn_e + 1); |
210 | strcat(mountdata, fullpath + (ref->path_consumed)); | 222 | strcat(mountdata, fullpath + ref->path_consumed); |
211 | } | 223 | } |
224 | --fullpath; | ||
212 | kfree(fullpath); | 225 | kfree(fullpath); |
213 | 226 | ||
214 | /*cFYI(1,("%s: parent mountdata: %s", __func__,sb_mountdata));*/ | 227 | /*cFYI(1,("%s: parent mountdata: %s", __func__,sb_mountdata));*/ |
@@ -217,6 +230,11 @@ static char *compose_mount_options(const char *sb_mountdata, | |||
217 | compose_mount_options_out: | 230 | compose_mount_options_out: |
218 | kfree(srvIP); | 231 | kfree(srvIP); |
219 | return mountdata; | 232 | return mountdata; |
233 | |||
234 | compose_mount_options_err: | ||
235 | kfree(mountdata); | ||
236 | mountdata = ERR_PTR(rc); | ||
237 | goto compose_mount_options_out; | ||
220 | } | 238 | } |
221 | 239 | ||
222 | 240 | ||
@@ -309,13 +327,19 @@ cifs_dfs_follow_mountpoint(struct dentry *dentry, struct nameidata *nd) | |||
309 | goto out_err; | 327 | goto out_err; |
310 | } | 328 | } |
311 | 329 | ||
330 | /* | ||
331 | * The MSDFS spec states that paths in DFS referral requests and | ||
332 | * responses must be prefixed by a single '\' character instead of | ||
333 | * the double backslashes usually used in the UNC. This function | ||
334 | * gives us the latter, so we must adjust the result. | ||
335 | */ | ||
312 | full_path = build_path_from_dentry(dentry); | 336 | full_path = build_path_from_dentry(dentry); |
313 | if (full_path == NULL) { | 337 | if (full_path == NULL) { |
314 | rc = -ENOMEM; | 338 | rc = -ENOMEM; |
315 | goto out_err; | 339 | goto out_err; |
316 | } | 340 | } |
317 | 341 | ||
318 | rc = get_dfs_path(xid, ses , full_path, cifs_sb->local_nls, | 342 | rc = get_dfs_path(xid, ses , full_path + 1, cifs_sb->local_nls, |
319 | &num_referrals, &referrals, | 343 | &num_referrals, &referrals, |
320 | cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR); | 344 | cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR); |
321 | 345 | ||