| Message ID | 20250324083755.12489-3-kwizart@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] Revert "drm/i915/gvt: Fix out-of-bounds buffer write into opregion->signature[]" | expand |
On Mon, 24 Mar 2025, Nicolas Chauvet <kwizart@gmail.com> wrote: > Enlarge the signature field to accept the string termination. > > Cc: stable@vger.kernel.org > Fixes: 93615d59912 ("Revert drm/i915/gvt: Fix out-of-bounds buffer write into opregion->signature[]") > Signed-off-by: Nicolas Chauvet <kwizart@gmail.com> Nope, can't do that. The packed struct is used for parsing data in memory. BR, Jani. > --- > drivers/gpu/drm/i915/gvt/opregion.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/gvt/opregion.c b/drivers/gpu/drm/i915/gvt/opregion.c > index 9a8ead6039e2..0f11cd6ba383 100644 > --- a/drivers/gpu/drm/i915/gvt/opregion.c > +++ b/drivers/gpu/drm/i915/gvt/opregion.c > @@ -43,7 +43,7 @@ > #define DEVICE_TYPE_EFP4 0x10 > > struct opregion_header { > - u8 signature[16]; > + u8 signature[32]; > u32 size; > u32 opregion_ver; > u8 bios_ver[32];
On Mon, 24 Mar 2025, Jani Nikula <jani.nikula@linux.intel.com> wrote: > On Mon, 24 Mar 2025, Nicolas Chauvet <kwizart@gmail.com> wrote: >> Enlarge the signature field to accept the string termination. >> >> Cc: stable@vger.kernel.org >> Fixes: 93615d59912 ("Revert drm/i915/gvt: Fix out-of-bounds buffer write into opregion->signature[]") >> Signed-off-by: Nicolas Chauvet <kwizart@gmail.com> > > Nope, can't do that. The packed struct is used for parsing data in > memory. Okay, so I mixed this up with display/intel_opregion.c. So it's not used for parsing here... but it's used for generating the data in memory, and we can't change the layout or contents. Regardless, we can't do either patch 2 or patch 3. BR, Jani. > > BR, > Jani. > > >> --- >> drivers/gpu/drm/i915/gvt/opregion.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/gvt/opregion.c b/drivers/gpu/drm/i915/gvt/opregion.c >> index 9a8ead6039e2..0f11cd6ba383 100644 >> --- a/drivers/gpu/drm/i915/gvt/opregion.c >> +++ b/drivers/gpu/drm/i915/gvt/opregion.c >> @@ -43,7 +43,7 @@ >> #define DEVICE_TYPE_EFP4 0x10 >> >> struct opregion_header { >> - u8 signature[16]; >> + u8 signature[32]; >> u32 size; >> u32 opregion_ver; >> u8 bios_ver[32];
Le lun. 24 mars 2025 à 10:34, Jani Nikula <jani.nikula@linux.intel.com> a écrit : > > On Mon, 24 Mar 2025, Jani Nikula <jani.nikula@linux.intel.com> wrote: > > On Mon, 24 Mar 2025, Nicolas Chauvet <kwizart@gmail.com> wrote: > >> Enlarge the signature field to accept the string termination. > >> > >> Cc: stable@vger.kernel.org > >> Fixes: 93615d59912 ("Revert drm/i915/gvt: Fix out-of-bounds buffer write into opregion->signature[]") > >> Signed-off-by: Nicolas Chauvet <kwizart@gmail.com> > > > > Nope, can't do that. The packed struct is used for parsing data in > > memory. > > Okay, so I mixed this up with display/intel_opregion.c. So it's not used > for parsing here... but it's used for generating the data in memory, and > we can't change the layout or contents. > > Regardless, we can't do either patch 2 or patch 3. Thanks for review. So does it means the only "Fix" is to drop Werror, at least for intel/gvt code ? I have CONFIG_DRM_I915_WERROR not set but CONFIG_DRM_WERROR=y, (same as Fedora) Unsure why the current Fedora kernel is unaffected by this build failure.
On Mon, 24 Mar 2025, Nicolas Chauvet <kwizart@gmail.com> wrote: > Le lun. 24 mars 2025 à 10:34, Jani Nikula > <jani.nikula@linux.intel.com> a écrit : >> >> On Mon, 24 Mar 2025, Jani Nikula <jani.nikula@linux.intel.com> wrote: >> > On Mon, 24 Mar 2025, Nicolas Chauvet <kwizart@gmail.com> wrote: >> >> Enlarge the signature field to accept the string termination. >> >> >> >> Cc: stable@vger.kernel.org >> >> Fixes: 93615d59912 ("Revert drm/i915/gvt: Fix out-of-bounds buffer write into opregion->signature[]") >> >> Signed-off-by: Nicolas Chauvet <kwizart@gmail.com> >> > >> > Nope, can't do that. The packed struct is used for parsing data in >> > memory. >> >> Okay, so I mixed this up with display/intel_opregion.c. So it's not used >> for parsing here... but it's used for generating the data in memory, and >> we can't change the layout or contents. >> >> Regardless, we can't do either patch 2 or patch 3. > > Thanks for review. > So does it means the only "Fix" is to drop Werror, at least for intel/gvt code ? Of course not. The fix is to address the warning. There's another thread about this, see my suggestion there [1]. BR, Jani. [1] https://lore.kernel.org/r/87r02ma8s3.fsf@intel.com > I have CONFIG_DRM_I915_WERROR not set but CONFIG_DRM_WERROR=y, (same as Fedora) > Unsure why the current Fedora kernel is unaffected by this build failure.
diff --git a/drivers/gpu/drm/i915/gvt/opregion.c b/drivers/gpu/drm/i915/gvt/opregion.c index 9a8ead6039e2..0f11cd6ba383 100644 --- a/drivers/gpu/drm/i915/gvt/opregion.c +++ b/drivers/gpu/drm/i915/gvt/opregion.c @@ -43,7 +43,7 @@ #define DEVICE_TYPE_EFP4 0x10 struct opregion_header { - u8 signature[16]; + u8 signature[32]; u32 size; u32 opregion_ver; u8 bios_ver[32];
Enlarge the signature field to accept the string termination. Cc: stable@vger.kernel.org Fixes: 93615d59912 ("Revert drm/i915/gvt: Fix out-of-bounds buffer write into opregion->signature[]") Signed-off-by: Nicolas Chauvet <kwizart@gmail.com> --- drivers/gpu/drm/i915/gvt/opregion.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)