On Tue, Sep 22, 2020 at 11:04:31AM +0000, Rojewski, Cezary wrote: > On 2020-09-22 11:04 AM, Andy Shevchenko wrote: > > On Mon, Sep 21, 2020 at 08:48:12PM +0000, Rojewski, Cezary wrote: > >> On 2020-09-21 8:41 PM, Andy Shevchenko wrote: > > ... > > >> While this should never happen (means user is somehow not making use of > >> officially released firmware binary), coredumps are useful only if you > >> have access to debug tools. In cases you'd mentioned, invalid hash would > >> have been dumped to coredump and crash reader simply wouldn't have been > >> able to navigate to actual build for it. The rest of the coredump is still > >> vital though. > >> > >> memcpy() could be gated behind an 'if' for safety if needed: > >> > >> info = cdev->ipc.config.fw_info; > >> eof = info + FW_INFO_SIZE_MAX; > >> /* navigate to fifth info segment (fw hash) */ > >> for (i = 0; i < 4 && info < eof; i++, info++) > >> /* info segments are separated by space each */ > >> if ((info = strnchr(info, eof - info, ' ')) == NULL) > >> break; > > > >> if (i == 4 && info < eof) > >> memcpy(pos, info, min(eof - info, CATPT_DUMP_HASH_SIZE)); > > > > And here basically enough check is info against NULL, right? > > Just try to look at different possibilities how to make code simpler and neater. > > > >> Didn't compile this, some typecheck fixes might be in order and so on. > > > > What you meant is: > if (i == 4 && !info) // instead of 'info < eof' > > right? Simply if (!info)... > If 4th space is last char in this string then info would end up being > non-NULL and equal to 'eof' and thus memcpy() would get invoked with > size=eof-info=0. ...which is not a problem. > catpt_coredump() is here to gather debug info for Intel folks to analyze > in case of critical error. In ideal world, it should not be required at > all as when we get here, there are bigger problems on our head. > Above solution is simpler than what is prevent in v7 while also > maintaining good readability - variable names - plus comments which you > suggested. Thanks! -- With Best Regards, Andy Shevchenko
