The others look fine - has anyone proved that this fixes a problem in the field (if so, I would like to push it upstream).<br><br><div class="gmail_quote">On Mon, Jan 11, 2010 at 3:02 PM, Jeff Layton <span dir="ltr"><<a href="mailto:jlayton@redhat.com">jlayton@redhat.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">Make sure the lengths in a QUERY_ALL_EAS reply don't make the parser walk<br>
off the end of the SMB.<br>
<br>
Signed-off-by: Jeff Layton <<a href="mailto:jlayton@redhat.com">jlayton@redhat.com</a>><br>
---<br>
fs/cifs/cifssmb.c | 30 +++++++++++++++++++++++++++---<br>
1 files changed, 27 insertions(+), 3 deletions(-)<br>
<br>
diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c<br>
index f5e1527..fb19f43 100644<br>
--- a/fs/cifs/cifssmb.c<br>
+++ b/fs/cifs/cifssmb.c<br>
@@ -5285,6 +5285,7 @@ CIFSSMBQAllEAs(const int xid, struct cifsTconInfo *tcon,<br>
struct fealist *ea_response_data;<br>
struct fea *temp_fea;<br>
char *temp_ptr;<br>
+ char *end_of_smb;<br>
__u16 params, byte_count, data_offset;<br>
<br>
cFYI(1, ("In Query All EAs path %s", searchName));<br>
@@ -5360,11 +5361,19 @@ QAllEAsRetry:<br>
data_offset = le16_to_cpu(pSMBr->t2.DataOffset);<br>
ea_response_data = (struct fealist *)<br>
(((char *) &pSMBr->hdr.Protocol) + data_offset);<br>
-<br>
list_len = le32_to_cpu(ea_response_data->list_len);<br>
cFYI(1, ("ea length %d", list_len));<br>
if (list_len <= 8) {<br>
cFYI(1, ("empty EA list returned from server"));<br>
+ rc = -EIO;<br>
+ goto QAllEAsOut;<br>
+ }<br>
+<br>
+ /* make sure list_len doesn't go past end of SMB */<br>
+ end_of_smb = (char *)pByteArea(&pSMBr->hdr) + BCC(&pSMBr->hdr);<br>
+ if ((char *)ea_response_data + list_len > end_of_smb) {<br>
+ cFYI(1, ("list_len goes beyond SMB"));<br>
+ rc = -EIO;<br>
goto QAllEAsOut;<br>
}<br>
<br>
@@ -5373,10 +5382,26 @@ QAllEAsRetry:<br>
temp_fea = ea_response_data->list;<br>
temp_ptr = (char *)temp_fea;<br>
while (list_len > 0) {<br>
+ __u8 name_len;<br>
__u16 value_len;<br>
list_len -= 4;<br>
temp_ptr += 4;<br>
- rc += temp_fea->name_len;<br>
+ /* make sure we can read name_len and value_len */<br>
+ if (temp_ptr > end_of_smb) {<br>
+ cFYI(1, ("fealist goes beyond end of SMB"));<br>
+ rc = -EIO;<br>
+ goto QAllEAsOut;<br>
+ }<br>
+<br>
+ name_len = temp_fea->name_len;<br>
+ value_len = le16_to_cpu(temp_fea->value_len);<br>
+ if (temp_ptr + name_len + value_len > end_of_smb) {<br>
+ cFYI(1, ("fealist goes beyond end of SMB"));<br>
+ rc = -EIO;<br>
+ goto QAllEAsOut;<br>
+ }<br>
+<br>
+ rc += name_len;<br>
/* account for prefix user. and trailing null */<br>
rc = rc + 5 + 1;<br>
if (rc < (int) buf_size) {<br>
@@ -5399,7 +5424,6 @@ QAllEAsRetry:<br>
/* account for trailing null */<br>
list_len--;<br>
temp_ptr++;<br>
- value_len = le16_to_cpu(temp_fea->value_len);<br>
list_len -= value_len;<br>
temp_ptr += value_len;<br>
/* BB check that temp_ptr is still<br>
<font color="#888888">--<br>
1.6.5.2<br>
<br>
</font></blockquote></div><br><br clear="all"><br>-- <br>Thanks,<br><br>Steve<br>