<!DOCTYPE html><html><head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> </head> <body> Hi Greg, <div class="moz-cite-prefix">On 27-07-2025 20:16, Usyskin, Alexander wrote: </div> <blockquote type="cite" cite="mid:CY5PR11MB6366AF03A73910CED71C7E37ED5BA@CY5PR11MB6366.namprd11.prod.outlook.com"> <blockquote type="cite"> <pre wrap="" class="moz-quote-pre">Subject: Re: [PATCH 2/9] mei: late_bind: add late binding component driver On Wed, Jul 16, 2025 at 02:26:26PM +0000, Usyskin, Alexander wrote: </pre> <blockquote type="cite"> <blockquote type="cite"> <blockquote type="cite"> <blockquote type="cite"> <blockquote type="cite"> <pre wrap="" class="moz-quote-pre">+ if (bytes < sizeof(rsp)) { + dev_err(dev, "bad response from the firmware: size </pre> </blockquote> </blockquote> </blockquote> </blockquote> </blockquote> <pre wrap="" class="moz-quote-pre">%zd < </pre> <blockquote type="cite"> <blockquote type="cite"> <blockquote type="cite"> <blockquote type="cite"> <pre wrap="" class="moz-quote-pre">%zu\n", </pre> <blockquote type="cite"> <pre wrap="" class="moz-quote-pre">+ bytes, sizeof(rsp)); + ret = -EPROTO; + goto end; + } </pre> </blockquote> <pre wrap="" class="moz-quote-pre"> Why not check this above when you check against the size of the </pre> </blockquote> </blockquote> </blockquote> </blockquote> <pre wrap="" class="moz-quote-pre">header? </pre> <blockquote type="cite"> <blockquote type="cite"> <blockquote type="cite"> <blockquote type="cite"> <pre wrap="" class="moz-quote-pre">You only need one size check, not 2. </pre> </blockquote> <pre wrap="" class="moz-quote-pre">Firmware may return only header with result field set without the data. </pre> </blockquote> <pre wrap="" class="moz-quote-pre"> Then the firmware is broken :) </pre> <blockquote type="cite"> <pre wrap="" class="moz-quote-pre">We are parsing the header first and then starting to parse data. If we check for whole message size at the beginning we'll miss the result </pre> </blockquote> <pre wrap="" class="moz-quote-pre">data. You mean you will make it harder to debug the firmware, as you will not be printing out the header information? Or something else? The bytes variable HAS to match the full structure size, not just the header size, according to this code. So just test for that and be done with it! </pre> </blockquote> <pre wrap="" class="moz-quote-pre"> The CSME firmware returns only command header if, like, command is not </pre> </blockquote> <pre wrap="" class="moz-quote-pre">recognised. </pre> <blockquote type="cite"> <pre wrap="" class="moz-quote-pre">This may happen because of firmware bug or for firmware is </pre> </blockquote> <pre wrap="" class="moz-quote-pre">configured/compiled </pre> <blockquote type="cite"> <pre wrap="" class="moz-quote-pre">that way. This seems reasonable for the complex protocols where firmware may not be aware of this particular command at all and have no idea what the data size </pre> </blockquote> <pre wrap="" class="moz-quote-pre">it </pre> <blockquote type="cite"> <pre wrap="" class="moz-quote-pre">should send in reply. Printing result from the header will allow us to understand either this is the </pre> </blockquote> <pre wrap="" class="moz-quote-pre">firmware </pre> <blockquote type="cite"> <pre wrap="" class="moz-quote-pre">problem or driver sent something wrong. </pre> </blockquote> <pre wrap="" class="moz-quote-pre"> Then make it obvious in your checking and in your error messages as to what you are doing here. Checking the size of the buffer in two different places, with different values is very odd, and deserves a lot of explaination. </pre> </blockquote> <pre wrap="" class="moz-quote-pre"> Is this addition /* * Received message size may be smaller than the full message size when * reply contains only MKHI header with result field set to the error code. * Check the header size and content first to output exact error, if needed, * and then process to the whole message. */ and remodelling error messages like "received less then header size from the firmware" made it clean for people not involved with our firmware? I'm too deep in this to judge the wording.</pre> </blockquote> <span style="color: rgb(66, 66, 66); font-family: "Segoe Sans", "Segoe UI", "Segoe UI Web (West European)", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; font-size: 16px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; white-space: normal; background-color: rgb(250, 250, 250); text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial; display: inline !important; float: none;">I'm planning to include the following code in the next revision. Does this change align with your recommendation? <blockquote> <span style="color: rgb(66, 66, 66); font-family: "Segoe Sans", "Segoe UI", "Segoe UI Web (West European)", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; font-size: 16px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; white-space: normal; background-color: rgb(250, 250, 250); text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial; display: inline !important; float: none;">+ /* + * Received message size may be smaller than the full message size when + * reply contains only MKHI header with result field set to the error code. + * Check the header size and content first to output exact error, if needed, + * and then process to the whole message. + */ if (bytes < sizeof(rsp.header)) { - dev_err(dev, "bad response header from the firmware: size %zd < %zu\n", + dev_err(dev, "received less than header size from the firmware: %zd < %zu\n", bytes, sizeof(rsp.header)); ret = -EPROTO; goto end; } if (mei_lb_check_response(dev, &rsp.header)) { - dev_err(dev, "bad result response from the firmware: 0x%x\n", + dev_err(dev, "bad response from the firmware: header: 0x%x\n", *(uint32_t *)&rsp.header); ret = -EPROTO; goto end; } if (bytes < sizeof(rsp)) { - dev_err(dev, "bad response from the firmware: size %zd < %zu\n", + dev_err(dev, "received less than message size from the firmware: %zd < %zu\n", bytes, sizeof(rsp)); ret = -EPROTO; goto end; </blockquote> Thanks, Badal <blockquote type="cite" cite="mid:CY5PR11MB6366AF03A73910CED71C7E37ED5BA@CY5PR11MB6366.namprd11.prod.outlook.com"> <pre wrap="" class="moz-quote-pre"> - - Thanks, Sasha </pre> </blockquote> </body> </html>