[PATCH 1/8] firmware: vpd: do not leave freed section attributes to the list

[PATCH 1/8] firmware: vpd: do not leave freed section attributes to the list

[Dmitry Torokhov]

We should only add section attribute to the list of section attributes
if we successfully created corresponding sysfs attribute.

Fixes: 049a59db34eb ("firmware: Google VPD sysfs driver")
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/firmware/google/vpd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/google/vpd.c b/drivers/firmware/google/vpd.c
index 1e7860f02f4f..23a24a6d02c2 100644
--- a/drivers/firmware/google/vpd.c
+++ b/drivers/firmware/google/vpd.c
@@ -136,12 +136,12 @@ static int vpd_section_attrib_add(const u8 *key, s32 key_len,
 	info->value = value;
 
 	INIT_LIST_HEAD(&info->list);
-	list_add_tail(&info->list, &sec->attribs);
 
 	ret = sysfs_create_bin_file(sec->kobj, &info->bin_attr);
 	if (ret)
 		goto free_info_key;
 
+	list_add_tail(&info->list, &sec->attribs);
 	return 0;
 
 free_info_key:
-- 
2.13.0.219.gdb65acc882-goog

[PATCH 2/8] firmware: vpd: use kdtrndup when copying section key

[Dmitry Torokhov]

Instead of open-coding kstrndup with kzalloc + memcpy, let's use
the helper.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/firmware/google/vpd.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/firmware/google/vpd.c b/drivers/firmware/google/vpd.c
index 23a24a6d02c2..acf3f3cdc3f8 100644
--- a/drivers/firmware/google/vpd.c
+++ b/drivers/firmware/google/vpd.c
@@ -118,14 +118,13 @@ static int vpd_section_attrib_add(const u8 *key, s32 key_len,
 	info = kzalloc(sizeof(*info), GFP_KERNEL);
 	if (!info)
 		return -ENOMEM;
-	info->key = kzalloc(key_len + 1, GFP_KERNEL);
+
+	info->key = kstrndup(key, key_len, GFP_KERNEL);
 	if (!info->key) {
 		ret = -ENOMEM;
 		goto free_info;
 	}
 
-	memcpy(info->key, key, key_len);
-
 	sysfs_bin_attr_init(&info->bin_attr);
 	info->bin_attr.attr.name = info->key;
 	info->bin_attr.attr.mode = 0444;
-- 
2.13.0.219.gdb65acc882-goog

Re: [PATCH 2/8] firmware: vpd: use kdtrndup when copying section key

[Guenter Roeck]

On Tue, May 23, 2017 at 5:07 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Instead of open-coding kstrndup with kzalloc + memcpy, let's use
> the helper.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Reviewed-by: Guenter Roeck <groeck@chromium.org>

> ---
>  drivers/firmware/google/vpd.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/firmware/google/vpd.c b/drivers/firmware/google/vpd.c
> index 23a24a6d02c2..acf3f3cdc3f8 100644
> --- a/drivers/firmware/google/vpd.c
> +++ b/drivers/firmware/google/vpd.c
> @@ -118,14 +118,13 @@ static int vpd_section_attrib_add(const u8 *key, s32 key_len,
>         info = kzalloc(sizeof(*info), GFP_KERNEL);
>         if (!info)
>                 return -ENOMEM;
> -       info->key = kzalloc(key_len + 1, GFP_KERNEL);
> +
> +       info->key = kstrndup(key, key_len, GFP_KERNEL);
>         if (!info->key) {
>                 ret = -ENOMEM;
>                 goto free_info;
>         }
>
> -       memcpy(info->key, key, key_len);
> -
>         sysfs_bin_attr_init(&info->bin_attr);
>         info->bin_attr.attr.name = info->key;
>         info->bin_attr.attr.mode = 0444;
> --
> 2.13.0.219.gdb65acc882-goog
>

[PATCH 3/8] firmware: vpd: avoid potential use-after-free when destroying section

[Dmitry Torokhov]

We should not free info->key before we remove sysfs attribute that uses
this data as its name.

Fixes: 049a59db34eb ("firmware: Google VPD sysfs driver")
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/firmware/google/vpd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/google/vpd.c b/drivers/firmware/google/vpd.c
index acf3f3cdc3f8..b3a6e918418b 100644
--- a/drivers/firmware/google/vpd.c
+++ b/drivers/firmware/google/vpd.c
@@ -157,8 +157,8 @@ static void vpd_section_attrib_destroy(struct vpd_section *sec)
 	struct vpd_attrib_info *temp;
 
 	list_for_each_entry_safe(info, temp, &sec->attribs, list) {
-		kfree(info->key);
 		sysfs_remove_bin_file(sec->kobj, &info->bin_attr);
+		kfree(info->key);
 		kfree(info);
 	}
 }
-- 
2.13.0.219.gdb65acc882-goog

Re: [PATCH 3/8] firmware: vpd: avoid potential use-after-free when destroying section

[Guenter Roeck]

On Tue, May 23, 2017 at 5:07 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> We should not free info->key before we remove sysfs attribute that uses
> this data as its name.
>
> Fixes: 049a59db34eb ("firmware: Google VPD sysfs driver")
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Reviewed-by: Guenter Roeck <groeck@chromium.org>

> ---
>  drivers/firmware/google/vpd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/google/vpd.c b/drivers/firmware/google/vpd.c
> index acf3f3cdc3f8..b3a6e918418b 100644
> --- a/drivers/firmware/google/vpd.c
> +++ b/drivers/firmware/google/vpd.c
> @@ -157,8 +157,8 @@ static void vpd_section_attrib_destroy(struct vpd_section *sec)
>         struct vpd_attrib_info *temp;
>
>         list_for_each_entry_safe(info, temp, &sec->attribs, list) {
> -               kfree(info->key);
>                 sysfs_remove_bin_file(sec->kobj, &info->bin_attr);
> +               kfree(info->key);
>                 kfree(info);
>         }
>  }
> --
> 2.13.0.219.gdb65acc882-goog
>

[PATCH 4/8] firmware: vpd: do not leak kobjects

[Dmitry Torokhov]

kobject_del() only unlinks kobject, we need to use kobject_put() to
make sure kobject will go away completely.

Fixes: 049a59db34eb ("firmware: Google VPD sysfs driver")
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/firmware/google/vpd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/google/vpd.c b/drivers/firmware/google/vpd.c
index b3a6e918418b..cb07a2984ece 100644
--- a/drivers/firmware/google/vpd.c
+++ b/drivers/firmware/google/vpd.c
@@ -243,7 +243,7 @@ static int vpd_section_destroy(struct vpd_section *sec)
 {
 	if (sec->enabled) {
 		vpd_section_attrib_destroy(sec);
-		kobject_del(sec->kobj);
+		kobject_put(sec->kobj);
 		sysfs_remove_bin_file(vpd_kobj, &sec->bin_attr);
 		kfree(sec->raw_name);
 		iounmap(sec->baseaddr);
@@ -330,7 +330,7 @@ static void __exit vpd_platform_exit(void)
 {
 	vpd_section_destroy(&ro_vpd);
 	vpd_section_destroy(&rw_vpd);
-	kobject_del(vpd_kobj);
+	kobject_put(vpd_kobj);
 }
 
 module_init(vpd_platform_init);
-- 
2.13.0.219.gdb65acc882-goog

Re: [PATCH 4/8] firmware: vpd: do not leak kobjects

[Guenter Roeck]

On Tue, May 23, 2017 at 5:07 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> kobject_del() only unlinks kobject, we need to use kobject_put() to
> make sure kobject will go away completely.
>
> Fixes: 049a59db34eb ("firmware: Google VPD sysfs driver")
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Reviewed-by: Guenter Roeck <groeck@chromium.org>

> ---
>  drivers/firmware/google/vpd.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/firmware/google/vpd.c b/drivers/firmware/google/vpd.c
> index b3a6e918418b..cb07a2984ece 100644
> --- a/drivers/firmware/google/vpd.c
> +++ b/drivers/firmware/google/vpd.c
> @@ -243,7 +243,7 @@ static int vpd_section_destroy(struct vpd_section *sec)
>  {
>         if (sec->enabled) {
>                 vpd_section_attrib_destroy(sec);
> -               kobject_del(sec->kobj);
> +               kobject_put(sec->kobj);
>                 sysfs_remove_bin_file(vpd_kobj, &sec->bin_attr);
>                 kfree(sec->raw_name);
>                 iounmap(sec->baseaddr);
> @@ -330,7 +330,7 @@ static void __exit vpd_platform_exit(void)
>  {
>         vpd_section_destroy(&ro_vpd);
>         vpd_section_destroy(&rw_vpd);
> -       kobject_del(vpd_kobj);
> +       kobject_put(vpd_kobj);
>  }
>
>  module_init(vpd_platform_init);
> --
> 2.13.0.219.gdb65acc882-goog
>

[PATCH 5/8] firmware: vpd: use kasprintf() when forming name of 'raw' attribute

[Dmitry Torokhov]

When creating name for the "raw" attribute, let's switch to using
kaspeintf() instead of doing it by hand. Also make sure we handle
errors.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/firmware/google/vpd.c | 31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/drivers/firmware/google/vpd.c b/drivers/firmware/google/vpd.c
index cb07a2984ece..3463b8037ed8 100644
--- a/drivers/firmware/google/vpd.c
+++ b/drivers/firmware/google/vpd.c
@@ -190,8 +190,7 @@ static int vpd_section_create_attribs(struct vpd_section *sec)
 static int vpd_section_init(const char *name, struct vpd_section *sec,
 			    phys_addr_t physaddr, size_t size)
 {
-	int ret;
-	int raw_len;
+	int err;
 
 	sec->baseaddr = memremap(physaddr, size, MEMREMAP_WB);
 	if (!sec->baseaddr)
@@ -200,10 +199,11 @@ static int vpd_section_init(const char *name, struct vpd_section *sec,
 	sec->name = name;
 
 	/* We want to export the raw partion with name ${name}_raw */
-	raw_len = strlen(name) + 5;
-	sec->raw_name = kzalloc(raw_len, GFP_KERNEL);
-	strncpy(sec->raw_name, name, raw_len);
-	strncat(sec->raw_name, "_raw", raw_len);
+	sec->raw_name = kasprintf(GFP_KERNEL, "%s_raw", name);
+	if (!sec->raw_name) {
+		err = -ENOMEM;
+		goto err_iounmap;
+	}
 
 	sysfs_bin_attr_init(&sec->bin_attr);
 	sec->bin_attr.attr.name = sec->raw_name;
@@ -212,14 +212,14 @@ static int vpd_section_init(const char *name, struct vpd_section *sec,
 	sec->bin_attr.read = vpd_section_read;
 	sec->bin_attr.private = sec;
 
-	ret = sysfs_create_bin_file(vpd_kobj, &sec->bin_attr);
-	if (ret)
-		goto free_sec;
+	err = sysfs_create_bin_file(vpd_kobj, &sec->bin_attr);
+	if (err)
+		goto err_free_raw_name;
 
 	sec->kobj = kobject_create_and_add(name, vpd_kobj);
 	if (!sec->kobj) {
-		ret = -EINVAL;
-		goto sysfs_remove;
+		err = -EINVAL;
+		goto err_sysfs_remove;
 	}
 
 	INIT_LIST_HEAD(&sec->attribs);
@@ -229,14 +229,13 @@ static int vpd_section_init(const char *name, struct vpd_section *sec,
 
 	return 0;
 
-sysfs_remove:
+err_sysfs_remove:
 	sysfs_remove_bin_file(vpd_kobj, &sec->bin_attr);
-
-free_sec:
+err_free_raw_name:
 	kfree(sec->raw_name);
+err_iounmap:
 	iounmap(sec->baseaddr);
-
-	return ret;
+	return err;
 }
 
 static int vpd_section_destroy(struct vpd_section *sec)
-- 
2.13.0.219.gdb65acc882-goog

Re: [PATCH 5/8] firmware: vpd: use kasprintf() when forming name of 'raw' attribute

[Guenter Roeck]

On Tue, May 23, 2017 at 5:07 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> When creating name for the "raw" attribute, let's switch to using
> kaspeintf() instead of doing it by hand. Also make sure we handle
> errors.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Reviewed-by: Guenter Roeck <groeck@chromium.org>

> ---
>  drivers/firmware/google/vpd.c | 31 +++++++++++++++----------------
>  1 file changed, 15 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/firmware/google/vpd.c b/drivers/firmware/google/vpd.c
> index cb07a2984ece..3463b8037ed8 100644
> --- a/drivers/firmware/google/vpd.c
> +++ b/drivers/firmware/google/vpd.c
> @@ -190,8 +190,7 @@ static int vpd_section_create_attribs(struct vpd_section *sec)
>  static int vpd_section_init(const char *name, struct vpd_section *sec,
>                             phys_addr_t physaddr, size_t size)
>  {
> -       int ret;
> -       int raw_len;
> +       int err;
>
>         sec->baseaddr = memremap(physaddr, size, MEMREMAP_WB);
>         if (!sec->baseaddr)
> @@ -200,10 +199,11 @@ static int vpd_section_init(const char *name, struct vpd_section *sec,
>         sec->name = name;
>
>         /* We want to export the raw partion with name ${name}_raw */
> -       raw_len = strlen(name) + 5;
> -       sec->raw_name = kzalloc(raw_len, GFP_KERNEL);
> -       strncpy(sec->raw_name, name, raw_len);
> -       strncat(sec->raw_name, "_raw", raw_len);
> +       sec->raw_name = kasprintf(GFP_KERNEL, "%s_raw", name);
> +       if (!sec->raw_name) {
> +               err = -ENOMEM;
> +               goto err_iounmap;
> +       }
>
>         sysfs_bin_attr_init(&sec->bin_attr);
>         sec->bin_attr.attr.name = sec->raw_name;
> @@ -212,14 +212,14 @@ static int vpd_section_init(const char *name, struct vpd_section *sec,
>         sec->bin_attr.read = vpd_section_read;
>         sec->bin_attr.private = sec;
>
> -       ret = sysfs_create_bin_file(vpd_kobj, &sec->bin_attr);
> -       if (ret)
> -               goto free_sec;
> +       err = sysfs_create_bin_file(vpd_kobj, &sec->bin_attr);
> +       if (err)
> +               goto err_free_raw_name;
>
>         sec->kobj = kobject_create_and_add(name, vpd_kobj);
>         if (!sec->kobj) {
> -               ret = -EINVAL;
> -               goto sysfs_remove;
> +               err = -EINVAL;
> +               goto err_sysfs_remove;
>         }
>
>         INIT_LIST_HEAD(&sec->attribs);
> @@ -229,14 +229,13 @@ static int vpd_section_init(const char *name, struct vpd_section *sec,
>
>         return 0;
>
> -sysfs_remove:
> +err_sysfs_remove:
>         sysfs_remove_bin_file(vpd_kobj, &sec->bin_attr);
> -
> -free_sec:
> +err_free_raw_name:
>         kfree(sec->raw_name);
> +err_iounmap:
>         iounmap(sec->baseaddr);
> -
> -       return ret;
> +       return err;
>  }
>
>  static int vpd_section_destroy(struct vpd_section *sec)
> --
> 2.13.0.219.gdb65acc882-goog
>

[PATCH 6/8] firmware: vpd: do not clear statically allocated data

[Dmitry Torokhov]

ro_vpd and rw_vpd are static module-scope variables that are guaranteed
to be initialized with zeroes, there is no need for explicit memset().

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/firmware/google/vpd.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/firmware/google/vpd.c b/drivers/firmware/google/vpd.c
index 3463b8037ed8..78945729388e 100644
--- a/drivers/firmware/google/vpd.c
+++ b/drivers/firmware/google/vpd.c
@@ -317,9 +317,6 @@ static int __init vpd_platform_init(void)
 	if (!vpd_kobj)
 		return -ENOMEM;
 
-	memset(&ro_vpd, 0, sizeof(ro_vpd));
-	memset(&rw_vpd, 0, sizeof(rw_vpd));
-
 	platform_driver_register(&vpd_driver);
 
 	return 0;
-- 
2.13.0.219.gdb65acc882-goog

Re: [PATCH 6/8] firmware: vpd: do not clear statically allocated data

[Guenter Roeck]

On Tue, May 23, 2017 at 5:07 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> ro_vpd and rw_vpd are static module-scope variables that are guaranteed
> to be initialized with zeroes, there is no need for explicit memset().
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Reviewed-by: Guenter Roeck <groeck@chromium.org>

> ---
>  drivers/firmware/google/vpd.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/drivers/firmware/google/vpd.c b/drivers/firmware/google/vpd.c
> index 3463b8037ed8..78945729388e 100644
> --- a/drivers/firmware/google/vpd.c
> +++ b/drivers/firmware/google/vpd.c
> @@ -317,9 +317,6 @@ static int __init vpd_platform_init(void)
>         if (!vpd_kobj)
>                 return -ENOMEM;
>
> -       memset(&ro_vpd, 0, sizeof(ro_vpd));
> -       memset(&rw_vpd, 0, sizeof(rw_vpd));
> -
>         platform_driver_register(&vpd_driver);
>
>         return 0;
> --
> 2.13.0.219.gdb65acc882-goog
>

[PATCH 7/8] firmware: vpd: remove platform driver

[Dmitry Torokhov]

There is no reason why VPD should register platform device and driver,
given that we do not use their respective kobjects to attach attributes,
nor do we need suspend/resume hooks, or any other features of device
core.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/firmware/google/vpd.c | 44 ++++++++++++++++---------------------------
 1 file changed, 16 insertions(+), 28 deletions(-)

diff --git a/drivers/firmware/google/vpd.c b/drivers/firmware/google/vpd.c
index 78945729388e..f4766bf6785a 100644
--- a/drivers/firmware/google/vpd.c
+++ b/drivers/firmware/google/vpd.c
@@ -22,8 +22,6 @@
 #include <linux/kobject.h>
 #include <linux/list.h>
 #include <linux/module.h>
-#include <linux/of_address.h>
-#include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/sysfs.h>
 
@@ -279,47 +277,37 @@ static int vpd_sections_init(phys_addr_t physaddr)
 		ret = vpd_section_init("rw", &rw_vpd,
 				       physaddr + sizeof(struct vpd_cbmem) +
 				       header.ro_size, header.rw_size);
-		if (ret)
+		if (ret) {
+			vpd_section_destroy(&ro_vpd);
 			return ret;
+		}
 	}
 
 	return 0;
 }
 
-static int vpd_probe(struct platform_device *pdev)
-{
-	int ret;
-	struct lb_cbmem_ref entry;
-
-	ret = coreboot_table_find(CB_TAG_VPD, &entry, sizeof(entry));
-	if (ret)
-		return ret;
-
-	return vpd_sections_init(entry.cbmem_addr);
-}
-
-static struct platform_driver vpd_driver = {
-	.probe = vpd_probe,
-	.driver = {
-		.name = "vpd",
-	},
-};
-
 static int __init vpd_platform_init(void)
 {
-	struct platform_device *pdev;
-
-	pdev = platform_device_register_simple("vpd", -1, NULL, 0);
-	if (IS_ERR(pdev))
-		return PTR_ERR(pdev);
+	struct lb_cbmem_ref entry;
+	int err;
 
 	vpd_kobj = kobject_create_and_add("vpd", firmware_kobj);
 	if (!vpd_kobj)
 		return -ENOMEM;
 
-	platform_driver_register(&vpd_driver);
+	err = coreboot_table_find(CB_TAG_VPD, &entry, sizeof(entry));
+	if (err)
+		goto err_kobject_put;
+
+	err = vpd_sections_init(entry.cbmem_addr);
+	if (err)
+		goto err_kobject_put;
 
 	return 0;
+
+err_kobject_put:
+	kobject_put(vpd_kobj);
+	return err;
 }
 
 static void __exit vpd_platform_exit(void)
-- 
2.13.0.219.gdb65acc882-goog

Re: [PATCH 7/8] firmware: vpd: remove platform driver

[Guenter Roeck]

On Tue, May 23, 2017 at 5:07 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> There is no reason why VPD should register platform device and driver,
> given that we do not use their respective kobjects to attach attributes,
> nor do we need suspend/resume hooks, or any other features of device
> core.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

One reason might be to use devm_ functions for memory allocations to
simplify error handling and cleanup. Without that, I agree.

Reviewed-by: Guenter Roeck <groeck@chromium.org>

> ---
>  drivers/firmware/google/vpd.c | 44 ++++++++++++++++---------------------------
>  1 file changed, 16 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/firmware/google/vpd.c b/drivers/firmware/google/vpd.c
> index 78945729388e..f4766bf6785a 100644
> --- a/drivers/firmware/google/vpd.c
> +++ b/drivers/firmware/google/vpd.c
> @@ -22,8 +22,6 @@
>  #include <linux/kobject.h>
>  #include <linux/list.h>
>  #include <linux/module.h>
> -#include <linux/of_address.h>
> -#include <linux/platform_device.h>
>  #include <linux/slab.h>
>  #include <linux/sysfs.h>
>
> @@ -279,47 +277,37 @@ static int vpd_sections_init(phys_addr_t physaddr)
>                 ret = vpd_section_init("rw", &rw_vpd,
>                                        physaddr + sizeof(struct vpd_cbmem) +
>                                        header.ro_size, header.rw_size);
> -               if (ret)
> +               if (ret) {
> +                       vpd_section_destroy(&ro_vpd);
>                         return ret;
> +               }
>         }
>
>         return 0;
>  }
>
> -static int vpd_probe(struct platform_device *pdev)
> -{
> -       int ret;
> -       struct lb_cbmem_ref entry;
> -
> -       ret = coreboot_table_find(CB_TAG_VPD, &entry, sizeof(entry));
> -       if (ret)
> -               return ret;
> -
> -       return vpd_sections_init(entry.cbmem_addr);
> -}
> -
> -static struct platform_driver vpd_driver = {
> -       .probe = vpd_probe,
> -       .driver = {
> -               .name = "vpd",
> -       },
> -};
> -
>  static int __init vpd_platform_init(void)
>  {
> -       struct platform_device *pdev;
> -
> -       pdev = platform_device_register_simple("vpd", -1, NULL, 0);
> -       if (IS_ERR(pdev))
> -               return PTR_ERR(pdev);
> +       struct lb_cbmem_ref entry;
> +       int err;
>
>         vpd_kobj = kobject_create_and_add("vpd", firmware_kobj);
>         if (!vpd_kobj)
>                 return -ENOMEM;
>
> -       platform_driver_register(&vpd_driver);
> +       err = coreboot_table_find(CB_TAG_VPD, &entry, sizeof(entry));
> +       if (err)
> +               goto err_kobject_put;
> +
> +       err = vpd_sections_init(entry.cbmem_addr);
> +       if (err)
> +               goto err_kobject_put;
>
>         return 0;
> +
> +err_kobject_put:
> +       kobject_put(vpd_kobj);
> +       return err;
>  }
>
>  static void __exit vpd_platform_exit(void)
> --
> 2.13.0.219.gdb65acc882-goog
>

Re: [PATCH 7/8] firmware: vpd: remove platform driver

[Julius Werner]

[-- Attachment #1: Type: text/plain, Size: 3772 bytes --]

I'm not a kernel expert so maybe I don't understand this right, but...
I think this might have been done this way to ensure that the driver
can get initialized correctly regardless of probe ordering.
coreboot_table_find() may fail with -EPROBE_DEFER if the
coreboot_table driver and its dependent (coreboot_table-acpi or
coreboot_table-of) haven't been probed yet. In that case we want the
VPD driver to wait and try again later, after they've been probed. I
believe with the way this used to be written the driver core did that
automatically for us, but with your change I'm not sure if it still
does.

On Wed, May 24, 2017 at 10:22 AM, Guenter Roeck <groeck@google.com> wrote:
> On Tue, May 23, 2017 at 5:07 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
>> There is no reason why VPD should register platform device and driver,
>> given that we do not use their respective kobjects to attach attributes,
>> nor do we need suspend/resume hooks, or any other features of device
>> core.
>>
>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>
> One reason might be to use devm_ functions for memory allocations to
> simplify error handling and cleanup. Without that, I agree.
>
> Reviewed-by: Guenter Roeck <groeck@chromium.org>
>
>> ---
>>  drivers/firmware/google/vpd.c | 44 ++++++++++++++++---------------------------
>>  1 file changed, 16 insertions(+), 28 deletions(-)
>>
>> diff --git a/drivers/firmware/google/vpd.c b/drivers/firmware/google/vpd.c
>> index 78945729388e..f4766bf6785a 100644
>> --- a/drivers/firmware/google/vpd.c
>> +++ b/drivers/firmware/google/vpd.c
>> @@ -22,8 +22,6 @@
>>  #include <linux/kobject.h>
>>  #include <linux/list.h>
>>  #include <linux/module.h>
>> -#include <linux/of_address.h>
>> -#include <linux/platform_device.h>
>>  #include <linux/slab.h>
>>  #include <linux/sysfs.h>
>>
>> @@ -279,47 +277,37 @@ static int vpd_sections_init(phys_addr_t physaddr)
>>                 ret = vpd_section_init("rw", &rw_vpd,
>>                                        physaddr + sizeof(struct vpd_cbmem) +
>>                                        header.ro_size, header.rw_size);
>> -               if (ret)
>> +               if (ret) {
>> +                       vpd_section_destroy(&ro_vpd);
>>                         return ret;
>> +               }
>>         }
>>
>>         return 0;
>>  }
>>
>> -static int vpd_probe(struct platform_device *pdev)
>> -{
>> -       int ret;
>> -       struct lb_cbmem_ref entry;
>> -
>> -       ret = coreboot_table_find(CB_TAG_VPD, &entry, sizeof(entry));
>> -       if (ret)
>> -               return ret;
>> -
>> -       return vpd_sections_init(entry.cbmem_addr);
>> -}
>> -
>> -static struct platform_driver vpd_driver = {
>> -       .probe = vpd_probe,
>> -       .driver = {
>> -               .name = "vpd",
>> -       },
>> -};
>> -
>>  static int __init vpd_platform_init(void)
>>  {
>> -       struct platform_device *pdev;
>> -
>> -       pdev = platform_device_register_simple("vpd", -1, NULL, 0);
>> -       if (IS_ERR(pdev))
>> -               return PTR_ERR(pdev);
>> +       struct lb_cbmem_ref entry;
>> +       int err;
>>
>>         vpd_kobj = kobject_create_and_add("vpd", firmware_kobj);
>>         if (!vpd_kobj)
>>                 return -ENOMEM;
>>
>> -       platform_driver_register(&vpd_driver);
>> +       err = coreboot_table_find(CB_TAG_VPD, &entry, sizeof(entry));
>> +       if (err)
>> +               goto err_kobject_put;
>> +
>> +       err = vpd_sections_init(entry.cbmem_addr);
>> +       if (err)
>> +               goto err_kobject_put;
>>
>>         return 0;
>> +
>> +err_kobject_put:
>> +       kobject_put(vpd_kobj);
>> +       return err;
>>  }
>>
>>  static void __exit vpd_platform_exit(void)
>> --
>> 2.13.0.219.gdb65acc882-goog
>>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4845 bytes --]

Re: [PATCH 7/8] firmware: vpd: remove platform driver

[Dmitry Torokhov]

On Wed, May 24, 2017 at 05:04:41PM -0700, Julius Werner wrote:
> I'm not a kernel expert so maybe I don't understand this right, but...
> I think this might have been done this way to ensure that the driver
> can get initialized correctly regardless of probe ordering.
> coreboot_table_find() may fail with -EPROBE_DEFER if the
> coreboot_table driver and its dependent (coreboot_table-acpi or
> coreboot_table-of) haven't been probed yet. In that case we want the
> VPD driver to wait and try again later, after they've been probed. I
> believe with the way this used to be written the driver core did that
> automatically for us, but with your change I'm not sure if it still
> does.

Hmm, I missed that. OK, I think we have a maze of platform drivers
there, I'll see what can be done.

> 
> On Wed, May 24, 2017 at 10:22 AM, Guenter Roeck <groeck@google.com> wrote:
> > On Tue, May 23, 2017 at 5:07 PM, Dmitry Torokhov
> > <dmitry.torokhov@gmail.com> wrote:
> >> There is no reason why VPD should register platform device and driver,
> >> given that we do not use their respective kobjects to attach attributes,
> >> nor do we need suspend/resume hooks, or any other features of device
> >> core.
> >>
> >> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> >
> > One reason might be to use devm_ functions for memory allocations to
> > simplify error handling and cleanup. Without that, I agree.
> >
> > Reviewed-by: Guenter Roeck <groeck@chromium.org>
> >
> >> ---
> >>  drivers/firmware/google/vpd.c | 44 ++++++++++++++++---------------------------
> >>  1 file changed, 16 insertions(+), 28 deletions(-)
> >>
> >> diff --git a/drivers/firmware/google/vpd.c b/drivers/firmware/google/vpd.c
> >> index 78945729388e..f4766bf6785a 100644
> >> --- a/drivers/firmware/google/vpd.c
> >> +++ b/drivers/firmware/google/vpd.c
> >> @@ -22,8 +22,6 @@
> >>  #include <linux/kobject.h>
> >>  #include <linux/list.h>
> >>  #include <linux/module.h>
> >> -#include <linux/of_address.h>
> >> -#include <linux/platform_device.h>
> >>  #include <linux/slab.h>
> >>  #include <linux/sysfs.h>
> >>
> >> @@ -279,47 +277,37 @@ static int vpd_sections_init(phys_addr_t physaddr)
> >>                 ret = vpd_section_init("rw", &rw_vpd,
> >>                                        physaddr + sizeof(struct vpd_cbmem) +
> >>                                        header.ro_size, header.rw_size);
> >> -               if (ret)
> >> +               if (ret) {
> >> +                       vpd_section_destroy(&ro_vpd);
> >>                         return ret;
> >> +               }
> >>         }
> >>
> >>         return 0;
> >>  }
> >>
> >> -static int vpd_probe(struct platform_device *pdev)
> >> -{
> >> -       int ret;
> >> -       struct lb_cbmem_ref entry;
> >> -
> >> -       ret = coreboot_table_find(CB_TAG_VPD, &entry, sizeof(entry));
> >> -       if (ret)
> >> -               return ret;
> >> -
> >> -       return vpd_sections_init(entry.cbmem_addr);
> >> -}
> >> -
> >> -static struct platform_driver vpd_driver = {
> >> -       .probe = vpd_probe,
> >> -       .driver = {
> >> -               .name = "vpd",
> >> -       },
> >> -};
> >> -
> >>  static int __init vpd_platform_init(void)
> >>  {
> >> -       struct platform_device *pdev;
> >> -
> >> -       pdev = platform_device_register_simple("vpd", -1, NULL, 0);
> >> -       if (IS_ERR(pdev))
> >> -               return PTR_ERR(pdev);
> >> +       struct lb_cbmem_ref entry;
> >> +       int err;
> >>
> >>         vpd_kobj = kobject_create_and_add("vpd", firmware_kobj);
> >>         if (!vpd_kobj)
> >>                 return -ENOMEM;
> >>
> >> -       platform_driver_register(&vpd_driver);
> >> +       err = coreboot_table_find(CB_TAG_VPD, &entry, sizeof(entry));
> >> +       if (err)
> >> +               goto err_kobject_put;
> >> +
> >> +       err = vpd_sections_init(entry.cbmem_addr);
> >> +       if (err)
> >> +               goto err_kobject_put;
> >>
> >>         return 0;
> >> +
> >> +err_kobject_put:
> >> +       kobject_put(vpd_kobj);
> >> +       return err;
> >>  }
> >>
> >>  static void __exit vpd_platform_exit(void)
> >> --
> >> 2.13.0.219.gdb65acc882-goog
> >>



-- 
Dmitry

[PATCH 8/8] firmware: vpd: fix confusion between memremap and iounmap

[Dmitry Torokhov]

We should not be using iounmap to unmap memory mapped with memremap.

This fixes following warnings generated by sparse in response to
incorrect type annotations:

  CHECK   drivers/firmware/google/vpd.c
drivers/firmware/google/vpd.c:235:20: warning: incorrect type in argument 1 (different address spaces)
drivers/firmware/google/vpd.c:235:20:    expected void volatile [noderef] <asn:2>*addr
drivers/firmware/google/vpd.c:235:20:    got char *baseaddr
drivers/firmware/google/vpd.c:246:28: warning: incorrect type in argument 1 (different address spaces)
drivers/firmware/google/vpd.c:246:28:    expected void volatile [noderef] <asn:2>*addr
drivers/firmware/google/vpd.c:246:28:    got char *baseaddr
drivers/firmware/google/vpd.c:258:14: warning: incorrect type in assignment (different address spaces)
drivers/firmware/google/vpd.c:258:14:    expected struct vpd_cbmem [noderef] <asn:2>*temp
drivers/firmware/google/vpd.c:258:14:    got void *

Fixes: 049a59db34eb ("firmware: Google VPD sysfs driver")
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/firmware/google/vpd.c | 39 ++++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/drivers/firmware/google/vpd.c b/drivers/firmware/google/vpd.c
index f4766bf6785a..b076210e38a2 100644
--- a/drivers/firmware/google/vpd.c
+++ b/drivers/firmware/google/vpd.c
@@ -200,7 +200,7 @@ static int vpd_section_init(const char *name, struct vpd_section *sec,
 	sec->raw_name = kasprintf(GFP_KERNEL, "%s_raw", name);
 	if (!sec->raw_name) {
 		err = -ENOMEM;
-		goto err_iounmap;
+		goto err_unmap;
 	}
 
 	sysfs_bin_attr_init(&sec->bin_attr);
@@ -231,8 +231,8 @@ static int vpd_section_init(const char *name, struct vpd_section *sec,
 	sysfs_remove_bin_file(vpd_kobj, &sec->bin_attr);
 err_free_raw_name:
 	kfree(sec->raw_name);
-err_iounmap:
-	iounmap(sec->baseaddr);
+err_unmap:
+	memunmap(sec->baseaddr);
 	return err;
 }
 
@@ -243,7 +243,7 @@ static int vpd_section_destroy(struct vpd_section *sec)
 		kobject_put(sec->kobj);
 		sysfs_remove_bin_file(vpd_kobj, &sec->bin_attr);
 		kfree(sec->raw_name);
-		iounmap(sec->baseaddr);
+		memunmap(sec->baseaddr);
 	}
 
 	return 0;
@@ -251,38 +251,39 @@ static int vpd_section_destroy(struct vpd_section *sec)
 
 static int vpd_sections_init(phys_addr_t physaddr)
 {
-	struct vpd_cbmem __iomem *temp;
-	struct vpd_cbmem header;
+	struct vpd_cbmem *header;
 	int ret = 0;
 
-	temp = memremap(physaddr, sizeof(struct vpd_cbmem), MEMREMAP_WB);
-	if (!temp)
+	header = memremap(physaddr, sizeof(struct vpd_cbmem), MEMREMAP_WB);
+	if (!header)
 		return -ENOMEM;
 
-	memcpy_fromio(&header, temp, sizeof(struct vpd_cbmem));
-	iounmap(temp);
-
-	if (header.magic != VPD_CBMEM_MAGIC)
-		return -ENODEV;
+	if (header->magic != VPD_CBMEM_MAGIC) {
+		ret = -ENODEV;
+		goto out;
+	}
 
-	if (header.ro_size) {
+	if (header->ro_size) {
 		ret = vpd_section_init("ro", &ro_vpd,
 				       physaddr + sizeof(struct vpd_cbmem),
-				       header.ro_size);
+				       header->ro_size);
 		if (ret)
-			return ret;
+			goto out;
 	}
 
-	if (header.rw_size) {
+	if (header->rw_size) {
 		ret = vpd_section_init("rw", &rw_vpd,
 				       physaddr + sizeof(struct vpd_cbmem) +
-				       header.ro_size, header.rw_size);
+						header->ro_size,
+				       header->rw_size);
 		if (ret) {
 			vpd_section_destroy(&ro_vpd);
-			return ret;
+			goto out;
 		}
 	}
 
+out:
+	memunmap(header);
 	return 0;
 }
 
-- 
2.13.0.219.gdb65acc882-goog

Re: [PATCH 8/8] firmware: vpd: fix confusion between memremap and iounmap

[Guenter Roeck]

On Tue, May 23, 2017 at 5:07 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> We should not be using iounmap to unmap memory mapped with memremap.
>
> This fixes following warnings generated by sparse in response to
> incorrect type annotations:
>
>   CHECK   drivers/firmware/google/vpd.c
> drivers/firmware/google/vpd.c:235:20: warning: incorrect type in argument 1 (different address spaces)
> drivers/firmware/google/vpd.c:235:20:    expected void volatile [noderef] <asn:2>*addr
> drivers/firmware/google/vpd.c:235:20:    got char *baseaddr
> drivers/firmware/google/vpd.c:246:28: warning: incorrect type in argument 1 (different address spaces)
> drivers/firmware/google/vpd.c:246:28:    expected void volatile [noderef] <asn:2>*addr
> drivers/firmware/google/vpd.c:246:28:    got char *baseaddr
> drivers/firmware/google/vpd.c:258:14: warning: incorrect type in assignment (different address spaces)
> drivers/firmware/google/vpd.c:258:14:    expected struct vpd_cbmem [noderef] <asn:2>*temp
> drivers/firmware/google/vpd.c:258:14:    got void *
>
> Fixes: 049a59db34eb ("firmware: Google VPD sysfs driver")
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Reviewed-by: Guenter Roeck <groeck@chromium.org>

> ---
>  drivers/firmware/google/vpd.c | 39 ++++++++++++++++++++-------------------
>  1 file changed, 20 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/firmware/google/vpd.c b/drivers/firmware/google/vpd.c
> index f4766bf6785a..b076210e38a2 100644
> --- a/drivers/firmware/google/vpd.c
> +++ b/drivers/firmware/google/vpd.c
> @@ -200,7 +200,7 @@ static int vpd_section_init(const char *name, struct vpd_section *sec,
>         sec->raw_name = kasprintf(GFP_KERNEL, "%s_raw", name);
>         if (!sec->raw_name) {
>                 err = -ENOMEM;
> -               goto err_iounmap;
> +               goto err_unmap;
>         }
>
>         sysfs_bin_attr_init(&sec->bin_attr);
> @@ -231,8 +231,8 @@ static int vpd_section_init(const char *name, struct vpd_section *sec,
>         sysfs_remove_bin_file(vpd_kobj, &sec->bin_attr);
>  err_free_raw_name:
>         kfree(sec->raw_name);
> -err_iounmap:
> -       iounmap(sec->baseaddr);
> +err_unmap:
> +       memunmap(sec->baseaddr);
>         return err;
>  }
>
> @@ -243,7 +243,7 @@ static int vpd_section_destroy(struct vpd_section *sec)
>                 kobject_put(sec->kobj);
>                 sysfs_remove_bin_file(vpd_kobj, &sec->bin_attr);
>                 kfree(sec->raw_name);
> -               iounmap(sec->baseaddr);
> +               memunmap(sec->baseaddr);
>         }
>
>         return 0;
> @@ -251,38 +251,39 @@ static int vpd_section_destroy(struct vpd_section *sec)
>
>  static int vpd_sections_init(phys_addr_t physaddr)
>  {
> -       struct vpd_cbmem __iomem *temp;
> -       struct vpd_cbmem header;
> +       struct vpd_cbmem *header;
>         int ret = 0;
>
> -       temp = memremap(physaddr, sizeof(struct vpd_cbmem), MEMREMAP_WB);
> -       if (!temp)
> +       header = memremap(physaddr, sizeof(struct vpd_cbmem), MEMREMAP_WB);
> +       if (!header)
>                 return -ENOMEM;
>
> -       memcpy_fromio(&header, temp, sizeof(struct vpd_cbmem));
> -       iounmap(temp);
> -
> -       if (header.magic != VPD_CBMEM_MAGIC)
> -               return -ENODEV;
> +       if (header->magic != VPD_CBMEM_MAGIC) {
> +               ret = -ENODEV;
> +               goto out;
> +       }
>
> -       if (header.ro_size) {
> +       if (header->ro_size) {
>                 ret = vpd_section_init("ro", &ro_vpd,
>                                        physaddr + sizeof(struct vpd_cbmem),
> -                                      header.ro_size);
> +                                      header->ro_size);
>                 if (ret)
> -                       return ret;
> +                       goto out;
>         }
>
> -       if (header.rw_size) {
> +       if (header->rw_size) {
>                 ret = vpd_section_init("rw", &rw_vpd,
>                                        physaddr + sizeof(struct vpd_cbmem) +
> -                                      header.ro_size, header.rw_size);
> +                                               header->ro_size,
> +                                      header->rw_size);
>                 if (ret) {
>                         vpd_section_destroy(&ro_vpd);
> -                       return ret;
> +                       goto out;
>                 }
>         }
>
> +out:
> +       memunmap(header);
>         return 0;
>  }
>
> --
> 2.13.0.219.gdb65acc882-goog
>

Re: [PATCH 8/8] firmware: vpd: fix confusion between memremap and iounmap

[Greg Kroah-Hartman]

On Tue, May 23, 2017 at 05:07:48PM -0700, Dmitry Torokhov wrote:
> We should not be using iounmap to unmap memory mapped with memremap.
> 
> This fixes following warnings generated by sparse in response to
> incorrect type annotations:
> 
>   CHECK   drivers/firmware/google/vpd.c
> drivers/firmware/google/vpd.c:235:20: warning: incorrect type in argument 1 (different address spaces)
> drivers/firmware/google/vpd.c:235:20:    expected void volatile [noderef] <asn:2>*addr
> drivers/firmware/google/vpd.c:235:20:    got char *baseaddr
> drivers/firmware/google/vpd.c:246:28: warning: incorrect type in argument 1 (different address spaces)
> drivers/firmware/google/vpd.c:246:28:    expected void volatile [noderef] <asn:2>*addr
> drivers/firmware/google/vpd.c:246:28:    got char *baseaddr
> drivers/firmware/google/vpd.c:258:14: warning: incorrect type in assignment (different address spaces)
> drivers/firmware/google/vpd.c:258:14:    expected struct vpd_cbmem [noderef] <asn:2>*temp
> drivers/firmware/google/vpd.c:258:14:    got void *
> 
> Fixes: 049a59db34eb ("firmware: Google VPD sysfs driver")
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Reviewed-by: Guenter Roeck <groeck@chromium.org>

This patch doesn't seem to apply to my tree :(

Re: [PATCH 1/8] firmware: vpd: do not leave freed section attributes to the list

[Guenter Roeck]

On Tue, May 23, 2017 at 5:07 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> We should only add section attribute to the list of section attributes
> if we successfully created corresponding sysfs attribute.
>
> Fixes: 049a59db34eb ("firmware: Google VPD sysfs driver")
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Reviewed-by: Guenter Roeck <groeck@chromium.org>

> ---
>  drivers/firmware/google/vpd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/google/vpd.c b/drivers/firmware/google/vpd.c
> index 1e7860f02f4f..23a24a6d02c2 100644
> --- a/drivers/firmware/google/vpd.c
> +++ b/drivers/firmware/google/vpd.c
> @@ -136,12 +136,12 @@ static int vpd_section_attrib_add(const u8 *key, s32 key_len,
>         info->value = value;
>
>         INIT_LIST_HEAD(&info->list);
> -       list_add_tail(&info->list, &sec->attribs);
>
>         ret = sysfs_create_bin_file(sec->kobj, &info->bin_attr);
>         if (ret)
>                 goto free_info_key;
>
> +       list_add_tail(&info->list, &sec->attribs);
>         return 0;
>
>  free_info_key:
> --
> 2.13.0.219.gdb65acc882-goog
>

Re: [PATCH 1/8] firmware: vpd: do not leave freed section attributes to the list

[Greg Kroah-Hartman]

On Tue, May 23, 2017 at 05:07:41PM -0700, Dmitry Torokhov wrote:
> We should only add section attribute to the list of section attributes
> if we successfully created corresponding sysfs attribute.
> 
> Fixes: 049a59db34eb ("firmware: Google VPD sysfs driver")
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Reviewed-by: Guenter Roeck <groeck@chromium.org>
> ---
>  drivers/firmware/google/vpd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Next time, can you split this up into 2 series, one for the current
kernel, and the rest for the "next" release?  I've tried to split them
up myself here, hopefully it works...

thanks,

greg k-h

Re: [PATCH 1/8] firmware: vpd: do not leave freed section attributes to the list

[Dmitry Torokhov]

On Thu, May 25, 2017 at 03:40:58PM +0200, Greg Kroah-Hartman wrote:
> On Tue, May 23, 2017 at 05:07:41PM -0700, Dmitry Torokhov wrote:
> > We should only add section attribute to the list of section attributes
> > if we successfully created corresponding sysfs attribute.
> > 
> > Fixes: 049a59db34eb ("firmware: Google VPD sysfs driver")
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > Reviewed-by: Guenter Roeck <groeck@chromium.org>
> > ---
> >  drivers/firmware/google/vpd.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Next time, can you split this up into 2 series, one for the current
> kernel, and the rest for the "next" release?  I've tried to split them
> up myself here, hopefully it works...

OK, I will. It is just I did not consider either of issues serious
enough so they could not wait for next release: failure to allocate tiny
amounts of memory is impossible to trigger with current kernels. Same
goes for the other patches. For example, one needs to not only manage to
get sysfs attribute creation to fail, but also then unload the driver,
to trigger the issue. Unlikely to happen in real life.

Thanks.

-- 
Dmitry

Re: [PATCH 1/8] firmware: vpd: do not leave freed section attributes to the list

[Greg Kroah-Hartman]

On Thu, May 25, 2017 at 09:35:25AM -0700, Dmitry Torokhov wrote:
> On Thu, May 25, 2017 at 03:40:58PM +0200, Greg Kroah-Hartman wrote:
> > On Tue, May 23, 2017 at 05:07:41PM -0700, Dmitry Torokhov wrote:
> > > We should only add section attribute to the list of section attributes
> > > if we successfully created corresponding sysfs attribute.
> > > 
> > > Fixes: 049a59db34eb ("firmware: Google VPD sysfs driver")
> > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > Reviewed-by: Guenter Roeck <groeck@chromium.org>
> > > ---
> > >  drivers/firmware/google/vpd.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > Next time, can you split this up into 2 series, one for the current
> > kernel, and the rest for the "next" release?  I've tried to split them
> > up myself here, hopefully it works...
> 
> OK, I will. It is just I did not consider either of issues serious
> enough so they could not wait for next release: failure to allocate tiny
> amounts of memory is impossible to trigger with current kernels. Same
> goes for the other patches. For example, one needs to not only manage to
> get sysfs attribute creation to fail, but also then unload the driver,
> to trigger the issue. Unlikely to happen in real life.

Ah, ok, that would have been good to know too :)