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">&lt;<a href="mailto:jlayton@redhat.com">jlayton@redhat.com</a>&gt;</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&#39;t make the parser walk<br>
off the end of the SMB.<br>
<br>
Signed-off-by: Jeff Layton &lt;<a href="mailto:jlayton@redhat.com">jlayton@redhat.com</a>&gt;<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, (&quot;In Query All EAs path %s&quot;, searchName));<br>
@@ -5360,11 +5361,19 @@ QAllEAsRetry:<br>
        data_offset = le16_to_cpu(pSMBr-&gt;t2.DataOffset);<br>
        ea_response_data = (struct fealist *)<br>
                                (((char *) &amp;pSMBr-&gt;hdr.Protocol) + data_offset);<br>
-<br>
        list_len = le32_to_cpu(ea_response_data-&gt;list_len);<br>
        cFYI(1, (&quot;ea length %d&quot;, list_len));<br>
        if (list_len &lt;= 8) {<br>
                cFYI(1, (&quot;empty EA list returned from server&quot;));<br>
+               rc = -EIO;<br>
+               goto QAllEAsOut;<br>
+       }<br>
+<br>
+       /* make sure list_len doesn&#39;t go past end of SMB */<br>
+       end_of_smb = (char *)pByteArea(&amp;pSMBr-&gt;hdr) + BCC(&amp;pSMBr-&gt;hdr);<br>
+       if ((char *)ea_response_data + list_len &gt; end_of_smb) {<br>
+               cFYI(1, (&quot;list_len goes beyond SMB&quot;));<br>
+               rc = -EIO;<br>
                goto QAllEAsOut;<br>
        }<br>
<br>
@@ -5373,10 +5382,26 @@ QAllEAsRetry:<br>
        temp_fea = ea_response_data-&gt;list;<br>
        temp_ptr = (char *)temp_fea;<br>
        while (list_len &gt; 0) {<br>
+               __u8 name_len;<br>
                __u16 value_len;<br>
                list_len -= 4;<br>
                temp_ptr += 4;<br>
-               rc += temp_fea-&gt;name_len;<br>
+               /* make sure we can read name_len and value_len */<br>
+               if (temp_ptr &gt; end_of_smb) {<br>
+                       cFYI(1, (&quot;fealist goes beyond end of SMB&quot;));<br>
+                       rc = -EIO;<br>
+                       goto QAllEAsOut;<br>
+               }<br>
+<br>
+               name_len = temp_fea-&gt;name_len;<br>
+               value_len = le16_to_cpu(temp_fea-&gt;value_len);<br>
+               if (temp_ptr + name_len + value_len &gt; end_of_smb) {<br>
+                       cFYI(1, (&quot;fealist goes beyond end of SMB&quot;));<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 &lt; (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-&gt;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>