Re: BUG printk with not null-terminated string in driver /drivers/acpi/osl.c

Re: BUG printk with not null-terminated string in driver /drivers/acpi/osl.c

[Björn Steinbrink]

On 2009.12.22 18:46:02 +0000, Alexander Strakh wrote:
>         In driver drivers/acpi/osl.c in function acpi_osi_setup:
> 1. If in line 555 *osi_additional_string == 0 then we goto line 1039

I guess that's meant to be line 1038?

> 2. In line 1039: if length of str > OSI_STRING_LENGTH_MAX then 
> osi_additional_string not have 0 at the end.
> 3. In line 1040 printk called with not null-terminated string.

That printk uses str, not osi_additional_string.

> diff --git a/./0000/drivers/acpi/osl.c b/./0001/drivers/acpi/osl.c
> index 02e8464..9c759f8 100644
> --- a/./0000/drivers/acpi/osl.c
> +++ b/./0001/drivers/acpi/osl.c
> @@ -1037,6 +1037,7 @@ int __init acpi_osi_setup(char *str)
>  		acpi_cmdline_osi_linux(1);	/* enable */
>  	} else if (*osi_additional_string == '\0') {
>  		strncpy(osi_additional_string, str, OSI_STRING_LENGTH_MAX);
> +		osi_additional_string[OSI_STRING_LENGTH_MAX - 1] = 0;

That will break the test in acpi_os_validate_interface for strings
longer than OSI_STRING_LENGTH_MAX (incl. the NUL byte). And that's the
only other user of osi_additional_string and doesn't require the
terminating NUL byte. So this shouldn't be needed.

Björn
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

BUG printk with not null-terminated string in driver /drivers/acpi/osl.c

[Alexander Strakh]

        In driver drivers/acpi/osl.c in function acpi_osi_setup:
1. If in line 555 *osi_additional_string == 0 then we goto line 1039
2. In line 1039: if length of str > OSI_STRING_LENGTH_MAX then 
osi_additional_string not have 0 at the end.
3. In line 1040 printk called with not null-terminated string.
1026 int __init acpi_osi_setup(char *str)
1027 {
1028         if (str == NULL || *str == '\0') {
1029                 printk(KERN_INFO PREFIX "_OSI method disabled\n");
1030                 acpi_gbl_create_osi_method = FALSE;
1031         } else if (!strcmp("!Linux", str)) {
1032                 acpi_cmdline_osi_linux(0);      /* !enable */
1033         } else if (*str == '!') {
1034                 if (acpi_osi_invalidate(++str) == AE_OK)
1035                         printk(KERN_INFO PREFIX "Deleted _OSI(%s)\n", 
str);
1036         } else if (!strcmp("Linux", str)) {
1037                 acpi_cmdline_osi_linux(1);      /* enable */
1038         } else if (*osi_additional_string == '\0') {
1039                 strncpy(osi_additional_string, str, 
OSI_STRING_LENGTH_MAX);
1040                 printk(KERN_INFO PREFIX "Added _OSI(%s)\n", str);
1041         }
1042
1043         return 1;
1044 }

Found by Linux Device Drivers Verification (Svace detector)

Add terminate symbol for string in any cases.

Signed-off-by: Alexander Strakh <strakh@ispras.ru>

---
diff --git a/./0000/drivers/acpi/osl.c b/./0001/drivers/acpi/osl.c
index 02e8464..9c759f8 100644
--- a/./0000/drivers/acpi/osl.c
+++ b/./0001/drivers/acpi/osl.c
@@ -1037,6 +1037,7 @@ int __init acpi_osi_setup(char *str)
 		acpi_cmdline_osi_linux(1);	/* enable */
 	} else if (*osi_additional_string == '\0') {
 		strncpy(osi_additional_string, str, OSI_STRING_LENGTH_MAX);
+		osi_additional_string[OSI_STRING_LENGTH_MAX - 1] = 0;
 		printk(KERN_INFO PREFIX "Added _OSI(%s)\n", str);
 	}