[PATCH v2 0/2] Fix some formatting problems

[PATCH v2 0/2] Fix some formatting problems

[Miko Larsson]

Hi,

This is v2 of the patch set I sent in two days ago. The subject is
arguably misleading, since we're doing a bit more than fixing some
"superficial formatting problems", but I wanted to keep the original
subject since this is a followup to the previous patch set.

This patch set adds SPDX license identifiers to zram_drv, and replaces
all uses of 'strlcpy()' with 'kmemdup_nul()' and 'strcpy()'.

Miko Larsson (2):
  zram: zram_drv: add SPDX license identifiers
  zram: zram_drv: replace 'strlcpy()'

 drivers/block/zram/zram_drv.c | 18 ++++++------------
 drivers/block/zram/zram_drv.h |  9 ++-------
 2 files changed, 8 insertions(+), 19 deletions(-)

-- 
2.34.1

[PATCH v2 1/2] zram: zram_drv: add SPDX license identifiers

[Miko Larsson]

zram_drv lacks an SPDX license identifier in both its source and in its
header, so we should add license identifiers based on the copyright info
provided by the initial comment block.

Signed-off-by: Miko Larsson <mikoxyzzz@gmail.com>
---
 drivers/block/zram/zram_drv.c | 9 ++-------
 drivers/block/zram/zram_drv.h | 9 ++-------
 2 files changed, 4 insertions(+), 14 deletions(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 25071126995b..4de6fe13edaf 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1,15 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
+
 /*
  * Compressed RAM block device
  *
  * Copyright (C) 2008, 2009, 2010  Nitin Gupta
  *               2012, 2013 Minchan Kim
- *
- * This code is released using a dual license strategy: BSD/GPL
- * You can choose the licence that better fits your requirements.
- *
- * Released under the terms of 3-clause BSD License
- * Released under the terms of GNU General Public License Version 2.0
- *
  */
 
 #define KMSG_COMPONENT "zram"
diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
index 80c3b43b4828..29e75e094a9d 100644
--- a/drivers/block/zram/zram_drv.h
+++ b/drivers/block/zram/zram_drv.h
@@ -1,15 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause */
+
 /*
  * Compressed RAM block device
  *
  * Copyright (C) 2008, 2009, 2010  Nitin Gupta
  *               2012, 2013 Minchan Kim
- *
- * This code is released using a dual license strategy: BSD/GPL
- * You can choose the licence that better fits your requirements.
- *
- * Released under the terms of 3-clause BSD License
- * Released under the terms of GNU General Public License Version 2.0
- *
  */
 
 #ifndef _ZRAM_DRV_H_
-- 
2.34.1

Re: [PATCH v2 1/2] zram: zram_drv: add SPDX license identifiers

[Christoph Hellwig]

On Fri, Dec 17, 2021 at 07:32:23AM +0100, Miko Larsson wrote:
> zram_drv lacks an SPDX license identifier in both its source and in its
> header, so we should add license identifiers based on the copyright info
> provided by the initial comment block.
> 
> Signed-off-by: Miko Larsson <mikoxyzzz@gmail.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH v2 1/2] zram: zram_drv: add SPDX license identifiers

[Joe Perches]

On Fri, 2021-12-17 at 07:32 +0100, Miko Larsson wrote:
> zram_drv lacks an SPDX license identifier in both its source and in its
> header, so we should add license identifiers based on the copyright info
> provided by the initial comment block.
> 
> Signed-off-by: Miko Larsson <mikoxyzzz@gmail.com>
> ---
>  drivers/block/zram/zram_drv.c | 9 ++-------
>  drivers/block/zram/zram_drv.h | 9 ++-------
>  2 files changed, 4 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
[]
> @@ -1,15 +1,10 @@
> +// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause

GPL v2 is a permissive license and this SPDX tag should probably be

// SPDX-License-Identifier: GPL-2.0-or-later or BSD-3-Clause

> +
>  /*
>   * Compressed RAM block device
>   *
>   * Copyright (C) 2008, 2009, 2010  Nitin Gupta
>   *               2012, 2013 Minchan Kim
> - *
> - * This code is released using a dual license strategy: BSD/GPL
> - * You can choose the licence that better fits your requirements.
> - *
> - * Released under the terms of 3-clause BSD License
> - * Released under the terms of GNU General Public License Version 2.0

Re: [PATCH v2 1/2] zram: zram_drv: add SPDX license identifiers

[Miko Larsson]

On Tue, 2021-12-21 at 01:33 -0800, Joe Perches wrote:
> GPL v2 is a permissive license and this SPDX tag should probably be
> 
> // SPDX-License-Identifier: GPL-2.0-or-later or BSD-3-Clause

It shouldn't be GPL-2.0-or-later, because the original copyright notice
doesn't have an "or later" clause.

Re: [PATCH v2 1/2] zram: zram_drv: add SPDX license identifiers

[Joe Perches]

(resending, original sent with an html part, unknown how)

On Fri, 2021-12-24 at 14:13 +0100, Miko Larsson wrote:
> On Tue, 2021-12-21 at 01:33 -0800, Joe Perches wrote:
> > GPL v2 is a permissive license and this SPDX tag should probably be
> > 
> > // SPDX-License-Identifier: GPL-2.0-or-later or BSD-3-Clause
> 
> It shouldn't be GPL-2.0-or-later, because the original copyright notice
> doesn't have an "or later" clause.

On its face, a GPL-2.0 license notice grant does _not_ need to
include any 'or later' content.

The GPL 2.0 license is permissive, it's at the _grantee's_ option.

I believe Nitin Gupta, the original author and submitter, should
declare whether or not he intended the license to include the
'or later' option.

Nitin?

Did you intend to use GPL-2.0 only and exclude GPL-3.0 or any later
version?

[PATCH v2 2/2] zram: zram_drv: replace 'strlcpy()'

[Miko Larsson]

'strlcpy()' shouldn't be used, and should be replaced with safer
alternatives. Cristoph Hellwig suggested [1] that 'kmemdup_nul()' should
be used in two cases instead of 'strscpy()', and that a regular
'strcpy()' should be used in the third case. [2][3]

[1] https://lore.kernel.org/all/YbsRlDYT2BfgrXRX@infradead.org/
[2] https://lore.kernel.org/all/Ybt8wY3U2ETjQijH@infradead.org/
[3] https://lore.kernel.org/all/Ybt8554NZpscKx2K@infradead.org/

Signed-off-by: Miko Larsson <mikoxyzzz@gmail.com>
---
 drivers/block/zram/zram_drv.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 4de6fe13edaf..c7c751e6ca2e 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -482,7 +482,7 @@ static ssize_t backing_dev_store(struct device *dev,
 	int err;
 	struct zram *zram = dev_to_zram(dev);
 
-	file_name = kmalloc(PATH_MAX, GFP_KERNEL);
+	file_name = kmemdup_nul(buf, PATH_MAX, GFP_KERNEL);
 	if (!file_name)
 		return -ENOMEM;
 
@@ -493,7 +493,6 @@ static ssize_t backing_dev_store(struct device *dev,
 		goto out;
 	}
 
-	strlcpy(file_name, buf, PATH_MAX);
 	/* ignore trailing newline */
 	sz = strlen(file_name);
 	if (sz > 0 && file_name[sz - 1] == '\n')
@@ -1024,10 +1023,10 @@ static ssize_t comp_algorithm_store(struct device *dev,
 		struct device_attribute *attr, const char *buf, size_t len)
 {
 	struct zram *zram = dev_to_zram(dev);
-	char compressor[ARRAY_SIZE(zram->compressor)];
+	char *compressor;
 	size_t sz;
 
-	strlcpy(compressor, buf, sizeof(compressor));
+	compressor = kmemdup_nul(buf, sizeof(zram->compressor), GFP_KERNEL);
 	/* ignore trailing newline */
 	sz = strlen(compressor);
 	if (sz > 0 && compressor[sz - 1] == '\n')
@@ -1981,7 +1980,7 @@ static int zram_add(void)
 	if (ret)
 		goto out_cleanup_disk;
 
-	strlcpy(zram->compressor, default_compressor, sizeof(zram->compressor));
+	strcpy(zram->compressor, default_compressor);
 
 	zram_debugfs_register(zram);
 	pr_info("Added device: %s\n", zram->disk->disk_name);
-- 
2.34.1

Re: [PATCH v2 2/2] zram: zram_drv: replace 'strlcpy()'

[Christoph Hellwig]

On Fri, Dec 17, 2021 at 07:32:24AM +0100, Miko Larsson wrote:
> 'strlcpy()' shouldn't be used, and should be replaced with safer
> alternatives. Cristoph Hellwig suggested [1] that 'kmemdup_nul()' should
> be used in two cases instead of 'strscpy()', and that a regular
> 'strcpy()' should be used in the third case. [2][3]

Not that strcpy isn't really safer, but if your plan is to get rid
of strlcpy I think it makes more sense there.

Reviewed-by: Christoph Hellwig <hch@lst.de>