Re: [PATCH v7 03/14] ASoC: Intel: catpt: Add IPC message handlers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux