[PATCH v2 1/2] string: add mem_is_zero() helper to check if memory area is all zeros

[PATCH v2 1/2] string: add mem_is_zero() helper to check if memory area is all zeros

[Jani Nikula]

Almost two thirds of the memchr_inv() usages check if the memory area is
all zeros, with no interest in where in the buffer the first non-zero
byte is located. Checking for !memchr_inv(s, 0, n) is also not very
intuitive or discoverable. Add an explicit mem_is_zero() helper for this
use case.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>

---

Cc: Kees Cook <keescook@chromium.org>
Cc: Andy Shevchenko <andy@kernel.org>
---
 include/linux/string.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/include/linux/string.h b/include/linux/string.h
index 9edace076ddb..5855c5626b4b 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -279,6 +279,18 @@ static inline void memcpy_flushcache(void *dst, const void *src, size_t cnt)
 void *memchr_inv(const void *s, int c, size_t n);
 char *strreplace(char *str, char old, char new);
 
+/**
+ * mem_is_zero - Check if an area of memory is all 0's.
+ * @s: The memory area
+ * @n: The size of the area
+ *
+ * Return: True if the area of memory is all 0's.
+ */
+static inline bool mem_is_zero(const void *s, size_t n)
+{
+	return !memchr_inv(s, 0, n);
+}
+
 extern void kfree_const(const void *x);
 
 extern char *kstrdup(const char *s, gfp_t gfp) __malloc;
-- 
2.39.2

[PATCH v2 2/2] drm: use mem_is_zero() instead of !memchr_inv(s, 0, n)

[Jani Nikula]

Use the mem_is_zero() helper where possible.

Conversion done using cocci:

| @@
| expression PTR;
| expression SIZE;
| @@
|
|   <...
| (
| - memchr_inv(PTR, 0, SIZE) == NULL
| + mem_is_zero(PTR, SIZE)
| |
| - !memchr_inv(PTR, 0, SIZE)
| + mem_is_zero(PTR, SIZE)
| |
| - memchr_inv(PTR, 0, SIZE)
| + !mem_is_zero(PTR, SIZE)
| )
|   ...>

Signed-off-by: Jani Nikula <jani.nikula@intel.com>

---

v2: Exclude GUID conversions, which are covered in [1].

[1] https://lore.kernel.org/r/20240812122312.1567046-1-jani.nikula@intel.com

Cc: Kees Cook <keescook@chromium.org>
Cc: Andy Shevchenko <andy@kernel.org>
---
 drivers/gpu/drm/drm_edid.c                           | 2 +-
 drivers/gpu/drm/i915/display/intel_dp.c              | 2 +-
 drivers/gpu/drm/i915/display/intel_opregion.c        | 2 +-
 drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c | 2 +-
 drivers/gpu/drm/imagination/pvr_device.h             | 2 +-
 drivers/gpu/drm/udl/udl_edid.c                       | 2 +-
 6 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index ff1e47a9c83e..855beafb76ff 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1817,7 +1817,7 @@ static int edid_block_tag(const void *_block)
 
 static bool edid_block_is_zero(const void *edid)
 {
-	return !memchr_inv(edid, 0, EDID_LENGTH);
+	return mem_is_zero(edid, EDID_LENGTH);
 }
 
 static bool drm_edid_eq(const struct drm_edid *drm_edid,
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 977f149551f6..6a0c7ae654f4 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -5184,7 +5184,7 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp)
 			ack[3] |= DP_TUNNELING_IRQ;
 		}
 
-		if (!memchr_inv(ack, 0, sizeof(ack)))
+		if (mem_is_zero(ack, sizeof(ack)))
 			break;
 
 		if (!intel_dp_ack_sink_irq_esi(intel_dp, ack))
diff --git a/drivers/gpu/drm/i915/display/intel_opregion.c b/drivers/gpu/drm/i915/display/intel_opregion.c
index dfa1d9f30d33..ff11836459de 100644
--- a/drivers/gpu/drm/i915/display/intel_opregion.c
+++ b/drivers/gpu/drm/i915/display/intel_opregion.c
@@ -1117,7 +1117,7 @@ const struct drm_edid *intel_opregion_get_edid(struct intel_connector *connector
 
 	/* Validity corresponds to number of 128-byte blocks */
 	len = (opregion->asle_ext->phed & ASLE_PHED_EDID_VALID_MASK) * 128;
-	if (!len || !memchr_inv(edid, 0, len))
+	if (!len || mem_is_zero(edid, len))
 		return NULL;
 
 	drm_edid = drm_edid_alloc(edid, len);
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
index 3527b8f446fe..2fda549dd82d 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
@@ -506,7 +506,7 @@ static int igt_dmabuf_export_vmap(void *arg)
 		goto out;
 	}
 
-	if (memchr_inv(ptr, 0, dmabuf->size)) {
+	if (!mem_is_zero(ptr, dmabuf->size)) {
 		pr_err("Exported object not initialised to zero!\n");
 		err = -EINVAL;
 		goto out;
diff --git a/drivers/gpu/drm/imagination/pvr_device.h b/drivers/gpu/drm/imagination/pvr_device.h
index ecdd5767d8ef..b574e23d484b 100644
--- a/drivers/gpu/drm/imagination/pvr_device.h
+++ b/drivers/gpu/drm/imagination/pvr_device.h
@@ -668,7 +668,7 @@ pvr_ioctl_union_padding_check(void *instance, size_t union_offset,
 	void *padding_start = ((u8 *)instance) + union_offset + member_size;
 	size_t padding_size = union_size - member_size;
 
-	return !memchr_inv(padding_start, 0, padding_size);
+	return mem_is_zero(padding_start, padding_size);
 }
 
 /**
diff --git a/drivers/gpu/drm/udl/udl_edid.c b/drivers/gpu/drm/udl/udl_edid.c
index d67e6bf1f2ae..12f48ae17073 100644
--- a/drivers/gpu/drm/udl/udl_edid.c
+++ b/drivers/gpu/drm/udl/udl_edid.c
@@ -69,7 +69,7 @@ bool udl_probe_edid(struct udl_device *udl)
 	 * The adapter sends all-zeros if no monitor has been
 	 * connected. We consider anything else a connection.
 	 */
-	return !!memchr_inv(hdr, 0, sizeof(hdr));
+	return !mem_is_zero(hdr, sizeof(hdr));
 }
 
 const struct drm_edid *udl_edid_read(struct drm_connector *connector)
-- 
2.39.2

Re: [PATCH v2 2/2] drm: use mem_is_zero() instead of !memchr_inv(s, 0, n)

[Kees Cook]

On Wed, Aug 14, 2024 at 01:00:35PM +0300, Jani Nikula wrote:
> Use the mem_is_zero() helper where possible.
> 
> Conversion done using cocci:
> 
> | @@
> | expression PTR;
> | expression SIZE;
> | @@
> |
> |   <...
> | (
> | - memchr_inv(PTR, 0, SIZE) == NULL
> | + mem_is_zero(PTR, SIZE)
> | |
> | - !memchr_inv(PTR, 0, SIZE)
> | + mem_is_zero(PTR, SIZE)
> | |
> | - memchr_inv(PTR, 0, SIZE)
> | + !mem_is_zero(PTR, SIZE)
> | )
> |   ...>
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Thanks for these patches! Since drm is the first user, feel free to
carry it there unless you'd prefer I carry it in my trees?

Reviewed-by: Kees Cook <kees@kernel.org>

-- 
Kees Cook

Re: [PATCH v2 2/2] drm: use mem_is_zero() instead of !memchr_inv(s, 0, n)

[Jani Nikula]

On Thu, 15 Aug 2024, Kees Cook <kees@kernel.org> wrote:
> On Wed, Aug 14, 2024 at 01:00:35PM +0300, Jani Nikula wrote:
>> Use the mem_is_zero() helper where possible.
>> 
>> Conversion done using cocci:
>> 
>> | @@
>> | expression PTR;
>> | expression SIZE;
>> | @@
>> |
>> |   <...
>> | (
>> | - memchr_inv(PTR, 0, SIZE) == NULL
>> | + mem_is_zero(PTR, SIZE)
>> | |
>> | - !memchr_inv(PTR, 0, SIZE)
>> | + mem_is_zero(PTR, SIZE)
>> | |
>> | - memchr_inv(PTR, 0, SIZE)
>> | + !mem_is_zero(PTR, SIZE)
>> | )
>> |   ...>
>> 
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> Thanks for these patches! Since drm is the first user, feel free to
> carry it there unless you'd prefer I carry it in my trees?
>
> Reviewed-by: Kees Cook <kees@kernel.org>

Thanks for the reviews. I've gone ahead and merged both to
drm-misc-next, heading for the next merge window.

BR,
Jani.


-- 
Jani Nikula, Intel

Re: [PATCH v2 1/2] string: add mem_is_zero() helper to check if memory area is all zeros

[Andy Shevchenko]

On Wed, Aug 14, 2024 at 1:00 PM Jani Nikula <jani.nikula@intel.com> wrote:
>
> Almost two thirds of the memchr_inv() usages check if the memory area is
> all zeros, with no interest in where in the buffer the first non-zero
> byte is located. Checking for !memchr_inv(s, 0, n) is also not very
> intuitive or discoverable. Add an explicit mem_is_zero() helper for this
> use case.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

...

>  void *memchr_inv(const void *s, int c, size_t n);

I would group them here, or move the above.

>  char *strreplace(char *str, char old, char new);
>
> +/**
> + * mem_is_zero - Check if an area of memory is all 0's.
> + * @s: The memory area
> + * @n: The size of the area
> + *
> + * Return: True if the area of memory is all 0's.
> + */
> +static inline bool mem_is_zero(const void *s, size_t n)
> +{
> +       return !memchr_inv(s, 0, n);
> +}

Reviewed-by: Andy Shevchenko <andy@kernel.org>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/2] string: add mem_is_zero() helper to check if memory area is all zeros

[Kees Cook]

On Wed, Aug 14, 2024 at 01:00:34PM +0300, Jani Nikula wrote:
> Almost two thirds of the memchr_inv() usages check if the memory area is
> all zeros, with no interest in where in the buffer the first non-zero
> byte is located. Checking for !memchr_inv(s, 0, n) is also not very
> intuitive or discoverable. Add an explicit mem_is_zero() helper for this
> use case.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Reviewed-by: Kees Cook <kees@kernel.org>

-- 
Kees Cook