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 <[email protected]>
---
Cc: Kees Cook <[email protected]>
Cc: Andy Shevchenko <[email protected]>
---
include/linux/string.h | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/include/linux/string.h b/include/linux/string.h
index 60168aa2af07..3da305dab927 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
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 <[email protected]>
---
Cc: Kees Cook <[email protected]>
Cc: Andy Shevchenko <[email protected]>
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
drivers/gpu/drm/display/drm_dp_mst_topology.c | 2 +-
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 +-
8 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 516eb3968e26..7d847021ddc9 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2432,7 +2432,7 @@ static void resume_mst_branch_status(struct drm_dp_mst_topology_mgr *mgr)
goto out_fail;
}
- if (memchr_inv(guid, 0, 16) == NULL) {
+ if (mem_is_zero(guid, 16)) {
tmp64 = get_jiffies_64();
memcpy(&guid[0], &tmp64, sizeof(u64));
memcpy(&guid[8], &tmp64, sizeof(u64));
diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
index 7f8e1cfbe19d..3d3097422235 100644
--- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
@@ -2697,7 +2697,7 @@ static bool drm_dp_validate_guid(struct drm_dp_mst_topology_mgr *mgr,
{
u64 salt;
- if (memchr_inv(guid, 0, 16))
+ if (!mem_is_zero(guid, 16))
return true;
salt = get_jiffies_64();
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index f68a41eeb1fa..9970c22f616b 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 c0a3b6d50681..be3685e115ab 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -5055,7 +5055,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 68bd5101ec89..293c4d920cf9 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 *intel_con
/* 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
On Mon, May 27, 2024 at 12:43 PM Jani Nikula <[email protected]> wrote:
>
> Use the mem_is_zero() helper where possible.
..
> - if (memchr_inv(guid, 0, 16) == NULL) {
> + if (mem_is_zero(guid, 16)) {
> tmp64 = get_jiffies_64();
> memcpy(&guid[0], &tmp64, sizeof(u64));
> memcpy(&guid[8], &tmp64, sizeof(u64));
What is the type of guid? Shouldn't it be guid_t with the respective
guid_is_null()
..
> - if (memchr_inv(guid, 0, 16))
> + if (!mem_is_zero(guid, 16))
> return true;
Ditto.
--
With Best Regards,
Andy Shevchenko
On Mon, May 27, 2024 at 12:43 PM Jani Nikula <[email protected]> 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.
..
> +static inline bool mem_is_zero(const void *s, size_t n)
> +{
> + return !memchr_inv(s, 0, n);
> +}
There are potential users for the 0xff check as well. Hence the
following question:
Are we going to have a new function per byte in question, or do we
come up with a common denominator, like mem_is_all_of(mem, byte)?
--
With Best Regards,
Andy Shevchenko
On Mon, 27 May 2024, Andy Shevchenko <[email protected]> wrote:
> On Mon, May 27, 2024 at 12:43 PM Jani Nikula <[email protected]> wrote:
>>
>> Use the mem_is_zero() helper where possible.
>
> ...
>
>> - if (memchr_inv(guid, 0, 16) == NULL) {
>> + if (mem_is_zero(guid, 16)) {
>> tmp64 = get_jiffies_64();
>> memcpy(&guid[0], &tmp64, sizeof(u64));
>> memcpy(&guid[8], &tmp64, sizeof(u64));
>
> What is the type of guid? Shouldn't it be guid_t with the respective
> guid_is_null()
I can leave out these parts of the patch.
BR,
Jani.
>
> ...
>
>> - if (memchr_inv(guid, 0, 16))
>> + if (!mem_is_zero(guid, 16))
>> return true;
>
> Ditto.
--
Jani Nikula, Intel
On Mon, 27 May 2024, Andy Shevchenko <[email protected]> wrote:
> On Mon, May 27, 2024 at 12:43 PM Jani Nikula <[email protected]> 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.
>
> ...
>
>> +static inline bool mem_is_zero(const void *s, size_t n)
>> +{
>> + return !memchr_inv(s, 0, n);
>> +}
>
> There are potential users for the 0xff check as well. Hence the
> following question:
> Are we going to have a new function per byte in question, or do we
> come up with a common denominator, like mem_is_all_of(mem, byte)?
No. As I wrote in the commit message rationale, "Almost two thirds of
the memchr_inv() usages check if the memory area is all zeros". This is
by far the most common use case of memchr_inv().
BR,
Jani.
--
Jani Nikula, Intel