Re: [LTP] [PATCH 2/2] Add test for CVE 2021-38198

[Martin Doucha <mdoucha@suse.cz>]

On 15. 03. 22 15:19, Richard Palethorpe wrote:
> Hi Martin,
> 
> Martin Doucha <mdoucha@suse.cz> writes:
>> +	if (isdigit(buf[0])) {
>> +		sscanf(buf, "%d", &ret);
> 
> checkpatch complains that the return value is not checked. Also it wants
> you to use tst_parse_int.

if (isdigit()) above ensures that the return value does not matter. But
I'll change it to tst_parse_int() in v2.

>> +static void disable_tdp(void)
>> +{
>> +	if (!access(TDP_MMU_SYSFILE, F_OK)) {
>> +		/* FIXME: Is this sufficient to disable TDP? */
> 
> What happens if this doesn't work and TDP is enabled? I seem to have it
> enabled and the test still passes even if I comment out the call
> to disable_tdp.
> 
> I'm wondering whether it will be easy to tell if a test failure is due
> to TDP or if it can result in silent false negatives?

AFAIK, the kernel bug was fixed before the tdp_mmu sysfile was
introduced. So somebody will have to revert the fix and run the test on
custom kernel to answer that question.

The bug is not reproducible with TDP enabled.

>> +	.save_restore = (const char *const []) {
>> +		"?/sys/module/kvm/parameters/tdp_mmu",
> 
> This needs updating to use struct tst_path_val.

Yes, the patch was sent before the .save_restore structure was changed.

-- 
Martin Doucha   mdoucha@suse.cz
QA Engineer for Software Maintenance
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp