[PATCH] drm/i915: Remove bogus __init annotation from DMI callbacks

[PATCH] drm/i915: Remove bogus __init annotation from DMI callbacks

[Mathias Krause]

The __init annotations for the DMI callback functions are wrong as this
code can be called even after the module has been initialized, e.g. like
this:

  # echo 1 > /sys/bus/pci/devices/0000:00:02.0/remove
  # modprobe i915
  # echo 1 > /sys/bus/pci/rescan

The first command will remove the PCI device from the kernel's device
list so the second command won't see it right away. But as it registers
a PCI driver it'll see it on the third command. If the system happens to
match one of the DMI table entries we'll try to call a function in long
released memory and generate an Oops, at best.

Fix this by removing the bogus annotation.

Modpost should have caught that one but it ignores section reference
mismatches from the .rodata section. :/

Fixes: 25e341cfc33d ("drm/i915: quirk away broken OpRegion VBT")
Fixes: 8ca4013d702d ("CHROMIUM: i915: Add DMI override to skip CRT...")
Fixes: 425d244c8670 ("drm/i915: ignore LVDS on intel graphics systems...")
Signed-off-by: Mathias Krause <minipli@googlemail.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Duncan Laurie <dlaurie@chromium.org>
Cc: Jarod Wilson <jarod@redhat.com>
Cc: Rusty Russell <rusty@rustcorp.com.au>	# Can modpost be fixed?
Cc: stable@vger.kernel.org
---

In the long run me might want to move the DMI tests to some __init code
to be able to mark the DMI tables as __initconst, thereby allowing to
release this memory after module initialization. That would safe us some
~11 kB of memory, as the DMI data shouldn't change at run-time.

 drivers/gpu/drm/i915/intel_bios.c |    2 +-
 drivers/gpu/drm/i915/intel_crt.c  |    2 +-
 drivers/gpu/drm/i915/intel_lvds.c |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index a66955037e4e..eee79e1c3222 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1123,7 +1123,7 @@ init_vbt_defaults(struct drm_i915_private *dev_priv)
 	}
 }
 
-static int __init intel_no_opregion_vbt_callback(const struct dmi_system_id *id)
+static int intel_no_opregion_vbt_callback(const struct dmi_system_id *id)
 {
 	DRM_DEBUG_KMS("Falling back to manually reading VBT from "
 		      "VBIOS ROM for %s\n",
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index e8abfce40976..9212e6504e0f 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -804,7 +804,7 @@ static const struct drm_encoder_funcs intel_crt_enc_funcs = {
 	.destroy = intel_encoder_destroy,
 };
 
-static int __init intel_no_crt_dmi_callback(const struct dmi_system_id *id)
+static int intel_no_crt_dmi_callback(const struct dmi_system_id *id)
 {
 	DRM_INFO("Skipping CRT initialization for %s\n", id->ident);
 	return 1;
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 881361c0f27e..fdf40267249c 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -538,7 +538,7 @@ static const struct drm_encoder_funcs intel_lvds_enc_funcs = {
 	.destroy = intel_encoder_destroy,
 };
 
-static int __init intel_no_lvds_dmi_callback(const struct dmi_system_id *id)
+static int intel_no_lvds_dmi_callback(const struct dmi_system_id *id)
 {
 	DRM_INFO("Skipping LVDS initialization for %s\n", id->ident);
 	return 1;
-- 
1.7.10.4

Re: [PATCH] drm/i915: Remove bogus __init annotation from DMI callbacks

[Jani Nikula]

On Wed, 27 Aug 2014, Mathias Krause <minipli@googlemail.com> wrote:
> The __init annotations for the DMI callback functions are wrong as this
> code can be called even after the module has been initialized, e.g. like
> this:
>
>   # echo 1 > /sys/bus/pci/devices/0000:00:02.0/remove
>   # modprobe i915
>   # echo 1 > /sys/bus/pci/rescan
>
> The first command will remove the PCI device from the kernel's device
> list so the second command won't see it right away. But as it registers
> a PCI driver it'll see it on the third command. If the system happens to
> match one of the DMI table entries we'll try to call a function in long
> released memory and generate an Oops, at best.
>
> Fix this by removing the bogus annotation.
>
> Modpost should have caught that one but it ignores section reference
> mismatches from the .rodata section. :/
>
> Fixes: 25e341cfc33d ("drm/i915: quirk away broken OpRegion VBT")
> Fixes: 8ca4013d702d ("CHROMIUM: i915: Add DMI override to skip CRT...")
> Fixes: 425d244c8670 ("drm/i915: ignore LVDS on intel graphics systems...")
> Signed-off-by: Mathias Krause <minipli@googlemail.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Duncan Laurie <dlaurie@chromium.org>
> Cc: Jarod Wilson <jarod@redhat.com>
> Cc: Rusty Russell <rusty@rustcorp.com.au>	# Can modpost be fixed?
> Cc: stable@vger.kernel.org

Nice catch! Thanks for the patch, pushed to drm-intel-fixes.

BR,
Jani.



> ---
>
> In the long run me might want to move the DMI tests to some __init code
> to be able to mark the DMI tables as __initconst, thereby allowing to
> release this memory after module initialization. That would safe us some
> ~11 kB of memory, as the DMI data shouldn't change at run-time.
>
>  drivers/gpu/drm/i915/intel_bios.c |    2 +-
>  drivers/gpu/drm/i915/intel_crt.c  |    2 +-
>  drivers/gpu/drm/i915/intel_lvds.c |    2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index a66955037e4e..eee79e1c3222 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1123,7 +1123,7 @@ init_vbt_defaults(struct drm_i915_private *dev_priv)
>  	}
>  }
>  
> -static int __init intel_no_opregion_vbt_callback(const struct dmi_system_id *id)
> +static int intel_no_opregion_vbt_callback(const struct dmi_system_id *id)
>  {
>  	DRM_DEBUG_KMS("Falling back to manually reading VBT from "
>  		      "VBIOS ROM for %s\n",
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index e8abfce40976..9212e6504e0f 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -804,7 +804,7 @@ static const struct drm_encoder_funcs intel_crt_enc_funcs = {
>  	.destroy = intel_encoder_destroy,
>  };
>  
> -static int __init intel_no_crt_dmi_callback(const struct dmi_system_id *id)
> +static int intel_no_crt_dmi_callback(const struct dmi_system_id *id)
>  {
>  	DRM_INFO("Skipping CRT initialization for %s\n", id->ident);
>  	return 1;
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 881361c0f27e..fdf40267249c 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -538,7 +538,7 @@ static const struct drm_encoder_funcs intel_lvds_enc_funcs = {
>  	.destroy = intel_encoder_destroy,
>  };
>  
> -static int __init intel_no_lvds_dmi_callback(const struct dmi_system_id *id)
> +static int intel_no_lvds_dmi_callback(const struct dmi_system_id *id)
>  {
>  	DRM_INFO("Skipping LVDS initialization for %s\n", id->ident);
>  	return 1;
> -- 
> 1.7.10.4
>

-- 
Jani Nikula, Intel Open Source Technology Center