On Wed, Jul 29, 2020 at 08:22:17AM +0200, Takashi Iwai wrote: > On Wed, 29 Jul 2020 03:17:39 +0200, > Luis Chamberlain wrote: > > > > Long ago Takashi had some points about this strategy breaking > > compressed file use. Was that considered? > > As long as I read the patch, it tries to skip both the compressed and > the fallback loading when FW_OPT_PARTIAL is set, which is good. > > However... > > > > @@ -771,18 +805,20 @@ _request_firmware(const struct firmware **firmware_p, const char *name, > > > } > > > > > > ret = _request_firmware_prepare(&fw, name, device, buf, size, > > > - opt_flags); > > > + offset, opt_flags); > > > if (ret <= 0) /* error or already assigned */ > > > goto out; > > > > > > ret = fw_get_filesystem_firmware(device, fw->priv, "", NULL); > > > -#ifdef CONFIG_FW_LOADER_COMPRESS > > > - if (ret == -ENOENT) > > > + > > > + /* Only full reads can support decompression, platform, and sysfs. */ > > > + if (!(opt_flags & FW_OPT_PARTIAL)) > > > + nondirect = true; > > > + > > > + if (ret == -ENOENT && nondirect) > > > ret = fw_get_filesystem_firmware(device, fw->priv, ".xz", > > > fw_decompress_xz); > > > -#endif > > ... by dropping this ifdef, the fw loader would try to access *.xz > file unnecessarily even if CONFIG_FW_LOADER_COMPRESS is disabled. Ah, good point. I'd added the -ENOENT fw_decompress_xz, but I take your point about the needless access. I will switch this back to an #ifdef. -- Kees Cook