Re: [PATCH] drm/panthor: Fix clang -Wunused-but-set-variable in tick_ctx_apply()
On Thu, Mar 28, 2024 at 9:22 AM Nathan Chancellor wrote:
>
> Clang warns (or errors with CONFIG_WERROR):
>
> drivers/gpu/drm/panthor/panthor_sched.c:2048:6: error: variable
> 'csg_mod_mask' set but not used [-Werror,-Wunused-but-set-variable]
>2048 | u32 csg_mod_mask = 0, free_csg_slots = 0;
> | ^
> 1 error generated.
>
> The variable is an artifact left over from refactoring that occurred
> during the development of the initial series for this driver. Remove it
> to resolve the warning.
Yep, makes sense.
>
> Fixes: de8548813824 ("drm/panthor: Add the scheduler logical block")
> Signed-off-by: Nathan Chancellor
Reviewed-by: Justin Stitt
> ---
> drivers/gpu/drm/panthor/panthor_sched.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c
> b/drivers/gpu/drm/panthor/panthor_sched.c
> index 5f7803b6fc48..e5a710f190d2 100644
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> @@ -2045,7 +2045,7 @@ tick_ctx_apply(struct panthor_scheduler *sched, struct
> panthor_sched_tick_ctx *c
> struct panthor_device *ptdev = sched->ptdev;
> struct panthor_csg_slot *csg_slot;
> int prio, new_csg_prio = MAX_CSG_PRIO, i;
> - u32 csg_mod_mask = 0, free_csg_slots = 0;
> + u32 free_csg_slots = 0;
> struct panthor_csg_slots_upd_ctx upd_ctx;
> int ret;
>
> @@ -2139,7 +2139,6 @@ tick_ctx_apply(struct panthor_scheduler *sched, struct
> panthor_sched_tick_ctx *c
>
> csg_iface = panthor_fw_get_csg_iface(ptdev, csg_id);
> csg_slot = >csg_slots[csg_id];
> - csg_mod_mask |= BIT(csg_id);
> group_bind_locked(group, csg_id);
> csg_slot_prog_locked(ptdev, csg_id, new_csg_prio--);
> csgs_upd_ctx_queue_reqs(ptdev, _ctx, csg_id,
>
> ---
> base-commit: d180649238f04183950d9c8a7d8a2c2f1788a89c
> change-id: 20240328-panthor-drop-csg_mod_mask-b4bbe317d690
>
> Best regards,
> --
> Nathan Chancellor
>
[PATCH] video/hdmi: prefer length specifier in format over string copying
strncpy() is deprecated for use on NUL-terminated destination strings
[1] and as such we should prefer more robust and less ambiguous string
interfaces.
It looks like the main use of strncpy() here is to limit the amount of
bytes printed from hdmi_log() by using a tmp buffer and limiting the
number of bytes copied. Really, we should use the %.s format
qualifier to achieve this.
Link:
https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings
[1]
Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-harden...@vger.kernel.org
Signed-off-by: Justin Stitt
---
Note: build-tested only.
Found with: $ rg "strncpy\("
---
drivers/video/hdmi.c | 10 ++
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
index 03c7f27dde49..ba301f3f4951 100644
--- a/drivers/video/hdmi.c
+++ b/drivers/video/hdmi.c
@@ -1310,17 +1310,11 @@ static void hdmi_spd_infoframe_log(const char *level,
struct device *dev,
const struct hdmi_spd_infoframe *frame)
{
- u8 buf[17];
-
hdmi_infoframe_log_header(level, dev,
(const struct hdmi_any_infoframe *)frame);
- memset(buf, 0, sizeof(buf));
-
- strncpy(buf, frame->vendor, 8);
- hdmi_log("vendor: %s\n", buf);
- strncpy(buf, frame->product, 16);
- hdmi_log("product: %s\n", buf);
+ hdmi_log("vendor: %.8s\n", frame->vendor);
+ hdmi_log("product: %.16s\n", frame->product);
hdmi_log("source device information: %s (0x%x)\n",
hdmi_spd_sdi_get_name(frame->sdi), frame->sdi);
}
---
base-commit: a4145ce1e7bc247fd6f2846e8699473448717b37
change-id: 20240320-strncpy-drivers-video-hdmi-c-bc18d585971f
Best regards,
--
Justin Stitt
[PATCH] fbdev: uvesafb: replace deprecated strncpy with strscpy_pad
strncpy() is deprecated for use on NUL-terminated destination strings
[1] and as such we should prefer more robust and less ambiguous string
interfaces.
We expect v86d_path to be NUL-terminated based on its use with the
C-string format specifier in printf-likes:
| pr_err("failed to execute %s\n", v86d_path);
and
| return snprintf(buf, PAGE_SIZE, "%s\n", v86d_path);
Let's also opt to pad v86d_path since it may get used in and around
userspace:
| return call_usermodehelper(v86d_path, argv, envp, UMH_WAIT_PROC);
Considering the above, strscpy_pad() is the best replacement as it
guarantees both NUL-termination and NUL-padding on the destination
buffer.
Note that this patch relies on the _new_ 2-argument versions of
strscpy() and strscpy_pad() introduced in Commit e6584c3964f2f ("string:
Allow 2-argument strscpy()").
Link:
https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings
[1]
Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-harden...@vger.kernel.org
Signed-off-by: Justin Stitt
---
Note: build-tested only.
Found with: $ rg "strncpy\("
---
drivers/video/fbdev/uvesafb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/uvesafb.c b/drivers/video/fbdev/uvesafb.c
index e1f421e91b4f..8b554fcf1bfb 100644
--- a/drivers/video/fbdev/uvesafb.c
+++ b/drivers/video/fbdev/uvesafb.c
@@ -1867,7 +1867,7 @@ static ssize_t v86d_show(struct device_driver *dev, char
*buf)
static ssize_t v86d_store(struct device_driver *dev, const char *buf,
size_t count)
{
- strncpy(v86d_path, buf, PATH_MAX - 1);
+ strscpy_pad(v86d_path, buf);
return count;
}
static DRIVER_ATTR_RW(v86d);
---
base-commit: a4145ce1e7bc247fd6f2846e8699473448717b37
change-id: 20240320-strncpy-drivers-video-fbdev-uvesafb-c-43668c4ef6c8
Best regards,
--
Justin Stitt
[PATCH] video: fbdev: fsl-diu-fb: replace deprecated strncpy with strscpy_pad
strncpy() is deprecated for use on NUL-terminated destination strings
[1] and as such we should prefer more robust and less ambiguous string
interfaces.
A better alternative is strscpy() as it guarantees NUL-termination on
the destination buffer.
Since we are eventually copying over to userspace, let's ensure we
NUL-pad the destination buffer by using the pad variant of strscpy.
- core/fb_chrdev.c:
234 | err = copy_to_user(>id, >id, sizeof(fix32->id));
Furthermore, we can use the new 2-argument variants of strscpy() and
strscpy_pad() introduced by Commit e6584c3964f2f ("string: Allow
2-argument strscpy()") to simplify the syntax even more.
Link:
https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings
[1]
Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-harden...@vger.kernel.org
Signed-off-by: Justin Stitt
---
Note: build-tested only.
Found with: $ rg "strncpy\("
---
drivers/video/fbdev/fsl-diu-fb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/fsl-diu-fb.c b/drivers/video/fbdev/fsl-diu-fb.c
index 0191141657fd..ea37a60da10c 100644
--- a/drivers/video/fbdev/fsl-diu-fb.c
+++ b/drivers/video/fbdev/fsl-diu-fb.c
@@ -787,7 +787,7 @@ static void set_fix(struct fb_info *info)
struct fb_var_screeninfo *var = >var;
struct mfb_info *mfbi = info->par;
- strncpy(fix->id, mfbi->id, sizeof(fix->id));
+ strscpy_pad(fix->id, mfbi->id);
fix->line_length = var->xres_virtual * var->bits_per_pixel / 8;
fix->type = FB_TYPE_PACKED_PIXELS;
fix->accel = FB_ACCEL_NONE;
---
base-commit: bf3a69c6861ff4dc7892d895c87074af7bc1c400
change-id: 20240319-strncpy-drivers-video-fbdev-fsl-diu-fb-c-b69036ceb3f4
Best regards,
--
Justin Stitt
Re: [PATCH] video: fbdev: au1200fb: replace deprecated strncpy with strscpy
Hi,
On Wed, Mar 20, 2024 at 12:56 AM Helge Deller wrote:
>
> On 3/19/24 00:46, Justin Stitt wrote:
> > strncpy() is deprecated for use on NUL-terminated destination strings
> > [1] and as such we should prefer more robust and less ambiguous string
> > interfaces.
> >
> > Let's use the new 2-argument strscpy() which guarantees NUL-termination
> > on the destination buffer while also simplifying the syntax. Note that
> > strscpy() will not NUL-pad the destination buffer like strncpy() does.
> >
> > However, the NUL-padding behavior of strncpy() is not required since
> > fbdev is already NUL-allocated from au1200fb_drv_probe() ->
> > frameuffer_alloc(), rendering any additional NUL-padding redundant.
> > | p = kzalloc(fb_info_size + size, GFP_KERNEL);
> >
> > Link:
> > https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings
> > [1]
> > Link:
> > https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
> > Link: https://github.com/KSPP/linux/issues/90
> > Cc: linux-harden...@vger.kernel.org
> > Signed-off-by: Justin Stitt
> > ---
> > Note: build-tested only.
> >
> > Found with: $ rg "strncpy\("
> > ---
> > drivers/video/fbdev/au1200fb.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/video/fbdev/au1200fb.c b/drivers/video/fbdev/au1200fb.c
> > index 6f20efc663d7..e718fea63662 100644
> > --- a/drivers/video/fbdev/au1200fb.c
> > +++ b/drivers/video/fbdev/au1200fb.c
> > @@ -1557,7 +1557,7 @@ static int au1200fb_init_fbinfo(struct
> > au1200fb_device *fbdev)
> > return ret;
> > }
> >
> > - strncpy(fbi->fix.id, "AU1200", sizeof(fbi->fix.id));
> > + strscpy(fbi->fix.id, "AU1200");
>
> I wonder if you really build-tested this, as this driver is for the mips
> architecture...
> And I don't see a strscpy() function which takes just 2 arguments.
> But I might be wrong
I did build successfully :thumbs_up:
Commit e6584c3964f2f ("string: Allow 2-argument strscpy()") introduced
this new strscpy() form; it is present in string.h on Linus' tree.
>
> Helge
Thanks
Justin
[PATCH] video: fbdev: au1200fb: replace deprecated strncpy with strscpy
strncpy() is deprecated for use on NUL-terminated destination strings
[1] and as such we should prefer more robust and less ambiguous string
interfaces.
Let's use the new 2-argument strscpy() which guarantees NUL-termination
on the destination buffer while also simplifying the syntax. Note that
strscpy() will not NUL-pad the destination buffer like strncpy() does.
However, the NUL-padding behavior of strncpy() is not required since
fbdev is already NUL-allocated from au1200fb_drv_probe() ->
frameuffer_alloc(), rendering any additional NUL-padding redundant.
| p = kzalloc(fb_info_size + size, GFP_KERNEL);
Link:
https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings
[1]
Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-harden...@vger.kernel.org
Signed-off-by: Justin Stitt
---
Note: build-tested only.
Found with: $ rg "strncpy\("
---
drivers/video/fbdev/au1200fb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/au1200fb.c b/drivers/video/fbdev/au1200fb.c
index 6f20efc663d7..e718fea63662 100644
--- a/drivers/video/fbdev/au1200fb.c
+++ b/drivers/video/fbdev/au1200fb.c
@@ -1557,7 +1557,7 @@ static int au1200fb_init_fbinfo(struct au1200fb_device
*fbdev)
return ret;
}
- strncpy(fbi->fix.id, "AU1200", sizeof(fbi->fix.id));
+ strscpy(fbi->fix.id, "AU1200");
fbi->fix.smem_start = fbdev->fb_phys;
fbi->fix.smem_len = fbdev->fb_len;
fbi->fix.type = FB_TYPE_PACKED_PIXELS;
---
base-commit: bf3a69c6861ff4dc7892d895c87074af7bc1c400
change-id: 20240318-strncpy-drivers-video-fbdev-au1200fb-c-7bc337998096
Best regards,
--
Justin Stitt
Re: [PATCH 7/9] drm: tests: Fix invalid printf format specifiers in KUnit tests
Hi,
On Wed, Feb 21, 2024 at 05:27:20PM +0800, David Gow wrote:
> The drm_buddy_test's alloc_contiguous test used a u64 for the page size,
> which was then updated to be an 'unsigned long' to avoid 64-bit
> multiplication division helpers.
>
> However, the variable is logged by some KUNIT_ASSERT_EQ_MSG() using the
> '%d' or '%llu' format specifiers, the former of which is always wrong,
> and the latter is no longer correct now that ps is no longer a u64. Fix
> these to all use '%lu'.
>
> Also, drm_mm_test calls KUNIT_FAIL() with an empty string as the
> message. gcc warns if a printf format string is empty (apparently), so
clang does too; under -Wformat-zero-length
> give these some more detailed error messages, which should be more
> useful anyway.
>
> Fixes: a64056bb5a32 ("drm/tests/drm_buddy: add alloc_contiguous test")
> Fixes: fca7526b7d89 ("drm/tests/drm_buddy: fix build failure on 32-bit
> targets")
> Fixes: fc8d29e298cf ("drm: selftest: convert drm_mm selftest to KUnit")
> Signed-off-by: David Gow
Reviewed-by: Justin Stitt
> ---
> drivers/gpu/drm/tests/drm_buddy_test.c | 14 +++---
> drivers/gpu/drm/tests/drm_mm_test.c| 6 +++---
> 2 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c
> b/drivers/gpu/drm/tests/drm_buddy_test.c
> index 8a464f7f4c61..3dbfa3078449 100644
> --- a/drivers/gpu/drm/tests/drm_buddy_test.c
> +++ b/drivers/gpu/drm/tests/drm_buddy_test.c
> @@ -55,30 +55,30 @@ static void drm_test_buddy_alloc_contiguous(struct kunit
> *test)
> KUNIT_ASSERT_FALSE_MSG(test,
> drm_buddy_alloc_blocks(, 0, mm_size,
> ps, ps, list, 0),
> -"buddy_alloc hit an error size=%d\n",
> +"buddy_alloc hit an error size=%lu\n",
> ps);
> } while (++i < n_pages);
>
> KUNIT_ASSERT_TRUE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size,
> 3 * ps, ps,
> ,
>
> DRM_BUDDY_CONTIGUOUS_ALLOCATION),
> -"buddy_alloc didn't error size=%d\n", 3 * ps);
> +"buddy_alloc didn't error size=%lu\n", 3 * ps);
>
> drm_buddy_free_list(, );
> KUNIT_ASSERT_TRUE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size,
> 3 * ps, ps,
> ,
>
> DRM_BUDDY_CONTIGUOUS_ALLOCATION),
> -"buddy_alloc didn't error size=%llu\n", 3 * ps);
> +"buddy_alloc didn't error size=%lu\n", 3 * ps);
> KUNIT_ASSERT_TRUE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size,
> 2 * ps, ps,
> ,
>
> DRM_BUDDY_CONTIGUOUS_ALLOCATION),
> -"buddy_alloc didn't error size=%llu\n", 2 * ps);
> +"buddy_alloc didn't error size=%lu\n", 2 * ps);
>
> drm_buddy_free_list(, );
> KUNIT_ASSERT_TRUE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size,
> 3 * ps, ps,
> ,
>
> DRM_BUDDY_CONTIGUOUS_ALLOCATION),
> -"buddy_alloc didn't error size=%llu\n", 3 * ps);
> +"buddy_alloc didn't error size=%lu\n", 3 * ps);
> /*
>* At this point we should have enough contiguous space for 2 blocks,
>* however they are never buddies (since we freed middle and right) so
> @@ -87,13 +87,13 @@ static void drm_test_buddy_alloc_contiguous(struct kunit
> *test)
> KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size,
> 2 * ps, ps,
> ,
>
> DRM_BUDDY_CONTIGUOUS_ALLOCATION),
> -"buddy_alloc hit an error size=%d\n", 2 * ps);
> +"buddy_alloc hit an error size=%lu\n", 2 * ps);
>
> drm_buddy_free_list(, );
> KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(, 0, mm_size,
> 3 * ps, ps,
> ,
>
Re: [PATCH 6/9] net: test: Fix printf format specifier in skb_segment kunit test
Hi,
On Wed, Feb 21, 2024 at 05:27:19PM +0800, David Gow wrote:
> KUNIT_FAIL() accepts a printf-style format string, but previously did
> not let gcc validate it with the __printf() attribute. The use of %lld
> for the result of PTR_ERR() is not correct.
>
> Instead, use %pe and pass the actual error pointer. printk() will format
> it correctly (and give a symbolic name rather than a number if
> available, which should make the output more readable, too).
>
> Fixes: b3098d32ed6e ("net: add skb_segment kunit test")
> Signed-off-by: David Gow
Looks good.
For those wondering, %pe has a special meaning in the kernel which can
be seen in lib/vsprintf.c.
Reviewed-by: Justin Stitt
> ---
> net/core/gso_test.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/core/gso_test.c b/net/core/gso_test.c
> index 4c2e77bd12f4..358c44680d91 100644
> --- a/net/core/gso_test.c
> +++ b/net/core/gso_test.c
> @@ -225,7 +225,7 @@ static void gso_test_func(struct kunit *test)
>
> segs = skb_segment(skb, features);
> if (IS_ERR(segs)) {
> - KUNIT_FAIL(test, "segs error %lld", PTR_ERR(segs));
> + KUNIT_FAIL(test, "segs error %pe", segs);
> goto free_gso_skb;
> } else if (!segs) {
> KUNIT_FAIL(test, "no segments");
> --
> 2.44.0.rc0.258.g7320e95886-goog
>
Thanks
Justin
Re: [PATCH 5/9] rtc: test: Fix invalid format specifier.
Hi,
On Wed, Feb 21, 2024 at 05:27:18PM +0800, David Gow wrote:
> 'days' is a s64 (from div_s64), and so should use a %lld specifier.
>
> This was found by extending KUnit's assertion macros to use gcc's
> __printf attribute.
>
> Fixes: 1d1bb12a8b18 ("rtc: Improve performance of rtc_time64_to_tm(). Add
> tests.")
> Signed-off-by: David Gow
Reviewed-by: Justin Stitt
> ---
> drivers/rtc/lib_test.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/rtc/lib_test.c b/drivers/rtc/lib_test.c
> index d5caf36c56cd..225c859d6da5 100644
> --- a/drivers/rtc/lib_test.c
> +++ b/drivers/rtc/lib_test.c
> @@ -54,7 +54,7 @@ static void rtc_time64_to_tm_test_date_range(struct kunit
> *test)
>
> days = div_s64(secs, 86400);
>
> - #define FAIL_MSG "%d/%02d/%02d (%2d) : %ld", \
> + #define FAIL_MSG "%d/%02d/%02d (%2d) : %lld", \
> year, month, mday, yday, days
>
> KUNIT_ASSERT_EQ_MSG(test, year - 1900, result.tm_year,
> FAIL_MSG);
> --
> 2.44.0.rc0.258.g7320e95886-goog
>
Thanks
Justin
Re: [PATCH 4/9] time: test: Fix incorrect format specifier
Hi,
On Wed, Feb 21, 2024 at 05:27:17PM +0800, David Gow wrote:
> 'days' is a s64 (from div_s64), and so should use a %lld specifier.
>
> This was found by extending KUnit's assertion macros to use gcc's
> __printf attribute.
>
> Fixes: 276010551664 ("time: Improve performance of time64_to_tm()")
> Signed-off-by: David Gow
Reviewed-by: Justin Stitt
> ---
> kernel/time/time_test.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/time/time_test.c b/kernel/time/time_test.c
> index ca058c8af6ba..3e5d422dd15c 100644
> --- a/kernel/time/time_test.c
> +++ b/kernel/time/time_test.c
> @@ -73,7 +73,7 @@ static void time64_to_tm_test_date_range(struct kunit *test)
>
> days = div_s64(secs, 86400);
>
> - #define FAIL_MSG "%05ld/%02d/%02d (%2d) : %ld", \
> + #define FAIL_MSG "%05ld/%02d/%02d (%2d) : %lld", \
> year, month, mdday, yday, days
>
> KUNIT_ASSERT_EQ_MSG(test, year - 1900, result.tm_year,
> FAIL_MSG);
> --
> 2.44.0.rc0.258.g7320e95886-goog
>
Thanks
Justin
Re: [PATCH 3/9] lib: memcpy_kunit: Fix an invalid format specifier in an assertion msg
Hi,
On Wed, Feb 21, 2024 at 05:27:16PM +0800, David Gow wrote:
> The 'i' passed as an assertion message is a size_t, so should use '%zu',
> not '%d'.
>
> This was found by annotating the _MSG() variants of KUnit's assertions
> to let gcc validate the format strings.
>
> Fixes: bb95ebbe89a7 ("lib: Introduce CONFIG_MEMCPY_KUNIT_TEST")
> Signed-off-by: David Gow
> ---
Reviewed-by: Justin Stitt
> lib/memcpy_kunit.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lib/memcpy_kunit.c b/lib/memcpy_kunit.c
> index 440aee705ccc..30e00ef0bf2e 100644
> --- a/lib/memcpy_kunit.c
> +++ b/lib/memcpy_kunit.c
> @@ -32,7 +32,7 @@ struct some_bytes {
> BUILD_BUG_ON(sizeof(instance.data) != 32); \
> for (size_t i = 0; i < sizeof(instance.data); i++) {\
> KUNIT_ASSERT_EQ_MSG(test, instance.data[i], v, \
> - "line %d: '%s' not initialized to 0x%02x @ %d (saw
> 0x%02x)\n", \
> + "line %d: '%s' not initialized to 0x%02x @ %zu (saw
> 0x%02x)\n", \
> __LINE__, #instance, v, i, instance.data[i]); \
> } \
> } while (0)
> @@ -41,7 +41,7 @@ struct some_bytes {
> BUILD_BUG_ON(sizeof(one) != sizeof(two)); \
> for (size_t i = 0; i < sizeof(one); i++) { \
> KUNIT_EXPECT_EQ_MSG(test, one.data[i], two.data[i], \
> - "line %d: %s.data[%d] (0x%02x) != %s.data[%d]
> (0x%02x)\n", \
> + "line %d: %s.data[%zu] (0x%02x) != %s.data[%zu]
> (0x%02x)\n", \
> __LINE__, #one, i, one.data[i], #two, i, two.data[i]); \
> } \
> kunit_info(test, "ok: " TEST_OP "() " name "\n"); \
> --
> 2.44.0.rc0.258.g7320e95886-goog
>
Thanks
Justin
Re: [PATCH 2/9] lib/cmdline: Fix an invalid format specifier in an assertion msg
Hi,
On Wed, Feb 21, 2024 at 05:27:15PM +0800, David Gow wrote:
> The correct format specifier for p - n (both p and n are pointers) is
> %td, as the type should be ptrdiff_t.
I think %tu is better. d specifies a signed type. I don't doubt that the
warning is fixed but I think %tu represents the type semantics here.
>
> This was discovered by annotating KUnit assertion macros with gcc's
> printf specifier, but note that gcc incorrectly suggested a %d or %ld
> specifier (depending on the pointer size of the architecture being
> built).
>
> Fixes: 0ea09083116d ("lib/cmdline: Allow get_options() to take 0 to validate
> the input")
> Signed-off-by: David Gow
> ---
> lib/cmdline_kunit.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/cmdline_kunit.c b/lib/cmdline_kunit.c
> index d4572dbc9145..705b82736be0 100644
> --- a/lib/cmdline_kunit.c
> +++ b/lib/cmdline_kunit.c
> @@ -124,7 +124,7 @@ static void cmdline_do_one_range_test(struct kunit *test,
> const char *in,
> n, e[0], r[0]);
>
> p = memchr_inv([1], 0, sizeof(r) - sizeof(r[0]));
> - KUNIT_EXPECT_PTR_EQ_MSG(test, p, NULL, "in test %u at %u out of bound",
> n, p - r);
> + KUNIT_EXPECT_PTR_EQ_MSG(test, p, NULL, "in test %u at %td out of
> bound", n, p - r);
> }
>
> static void cmdline_test_range(struct kunit *test)
> --
> 2.44.0.rc0.258.g7320e95886-goog
>
Thanks
Justin
Re: [PATCH 1/9] kunit: test: Log the correct filter string in executor_test
Hi,
On Wed, Feb 21, 2024 at 05:27:14PM +0800, David Gow wrote:
> KUnit's executor_test logs the filter string in KUNIT_ASSERT_EQ_MSG(),
> but passed a random character from the filter, rather than the whole
> string.
>
> This was found by annotating KUNIT_ASSERT_EQ_MSG() to let gcc validate
> the format string.
>
> Fixes: 76066f93f1df ("kunit: add tests for filtering attributes")
> Signed-off-by: David Gow
Reviewed-by: Justin Stitt
> ---
> lib/kunit/executor_test.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/kunit/executor_test.c b/lib/kunit/executor_test.c
> index 22d4ee86dbed..3f7f967e3688 100644
> --- a/lib/kunit/executor_test.c
> +++ b/lib/kunit/executor_test.c
> @@ -129,7 +129,7 @@ static void parse_filter_attr_test(struct kunit *test)
> GFP_KERNEL);
> for (j = 0; j < filter_count; j++) {
> parsed_filters[j] = kunit_next_attr_filter(, );
> - KUNIT_ASSERT_EQ_MSG(test, err, 0, "failed to parse filter
> '%s'", filters[j]);
> + KUNIT_ASSERT_EQ_MSG(test, err, 0, "failed to parse filter from
> '%s'", filters);
> }
>
> KUNIT_EXPECT_STREQ(test, kunit_attr_filter_name(parsed_filters[0]),
> "speed");
> --
> 2.44.0.rc0.258.g7320e95886-goog
>
Thanks
Justin
Re: [PATCH 9/9] kunit: Annotate _MSG assertion variants with gnu printf specifiers
Hi,
On Wed, Feb 21, 2024 at 05:27:22PM +0800, David Gow wrote:
> KUnit's assertion macros have variants which accept a printf format
> string, to allow tests to specify a more detailed message on failure.
> These (and the related KUNIT_FAIL() macro) ultimately wrap the
> __kunit_do_failed_assertion() function, which accepted a printf format
> specifier, but did not have the __printf attribute, so gcc couldn't warn
> on incorrect agruments.
>
> It turns out there were quite a few tests with such incorrect arguments.
>
> Add the __printf() specifier now that we've fixed these errors, to
> prevent them from recurring.
>
> Suggested-by: Linus Torvalds
> Signed-off-by: David Gow
Reviewed-by: Justin Stitt
> ---
> include/kunit/test.h | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index fcb4a4940ace..61637ef32302 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -579,12 +579,12 @@ void __printf(2, 3) kunit_log_append(struct
> string_stream *log, const char *fmt,
>
> void __noreturn __kunit_abort(struct kunit *test);
>
> -void __kunit_do_failed_assertion(struct kunit *test,
> -const struct kunit_loc *loc,
> -enum kunit_assert_type type,
> -const struct kunit_assert *assert,
> -assert_format_t assert_format,
> -const char *fmt, ...);
> +void __printf(6, 7) __kunit_do_failed_assertion(struct kunit *test,
> + const struct kunit_loc *loc,
> + enum kunit_assert_type type,
> + const struct kunit_assert
> *assert,
> + assert_format_t assert_format,
> + const char *fmt, ...);
>
> #define _KUNIT_FAILED(test, assert_type, assert_class, assert_format,
> INITIALIZER, fmt, ...) do { \
> static const struct kunit_loc __loc = KUNIT_CURRENT_LOC; \
> --
> 2.44.0.rc0.258.g7320e95886-goog
>
Thanks
Justin
[PATCH v2] drm/modes: replace deprecated strncpy with strscpy_pad
`strncpy` is deprecated for use on NUL-terminated destination strings
[1] and as such we should prefer more robust and less ambiguous string
interfaces.
We should NUL-pad as there are full struct copies happening in places:
| struct drm_mode_modeinfo umode;
|
| ...
| struct drm_property_blob *blob;
|
| drm_mode_convert_to_umode(, mode);
| blob = drm_property_create_blob(crtc->dev,
| sizeof(umode), );
A suitable replacement is `strscpy_pad` due to the fact that it
guarantees both NUL-termination and NUL-padding on the destination
buffer.
Additionally, replace size macro `DRM_DISPLAY_MODE_LEN` with sizeof() to
more directly tie the maximum buffer size to the destination buffer:
| struct drm_display_mode {
| ...
| char name[DRM_DISPLAY_MODE_LEN];
Link:
https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings
[1]
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-harden...@vger.kernel.org
Cc: Xu Panda
Signed-off-by: Justin Stitt
---
Changes in v2:
- use strscpy_pad (thanks Kees)
- rebase onto mainline
- Link to v1:
https://lore.kernel.org/r/20230914-strncpy-drivers-gpu-drm-drm_modes-c-v1-1-079b53255...@google.com
---
Note: build-tested only.
---
drivers/gpu/drm/drm_modes.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index ac9a406250c5..893f52ee4926 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -2617,8 +2617,7 @@ void drm_mode_convert_to_umode(struct drm_mode_modeinfo
*out,
break;
}
- strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN);
- out->name[DRM_DISPLAY_MODE_LEN-1] = 0;
+ strscpy_pad(out->name, in->name, sizeof(out->name));
}
/**
@@ -2659,8 +2658,7 @@ int drm_mode_convert_umode(struct drm_device *dev,
* useful for the kernel->userspace direction anyway.
*/
out->type = in->type & DRM_MODE_TYPE_ALL;
- strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN);
- out->name[DRM_DISPLAY_MODE_LEN-1] = 0;
+ strscpy_pad(out->name, in->name, sizeof(out->name));
/* Clearing picture aspect ratio bits from out flags,
* as the aspect-ratio information is not stored in
---
base-commit: 58720809f52779dc0f08e53e54b014209d13eebb
change-id: 20230914-strncpy-drivers-gpu-drm-drm_modes-c-a35d782cad01
Best regards,
--
Justin Stitt
[PATCH v2] drm/i915: refactor deprecated strncpy
`strncpy` is deprecated for use on NUL-terminated destination strings [1].
We should prefer more robust and less ambiguous string interfaces.
A suitable replacement is `strscpy` [2] due to the fact that it
guarantees NUL-termination on the destination buffer without
unnecessarily NUL-padding. `ctx` is zero allocated and as such strncpy's
NUL-padding behavior was strictly a performance hit which is now
resolved.
Link:
https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings
[1]
Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-harden...@vger.kernel.org
Signed-off-by: Justin Stitt
---
Changes in v2:
- drop the `... - 1` (thanks Kees)
- Link to v1:
https://lore.kernel.org/r/20230914-strncpy-drivers-gpu-drm-i915-gem-selftests-mock_context-c-v1-1-c3f92df94...@google.com
---
drivers/gpu/drm/i915/gem/selftests/mock_context.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gem/selftests/mock_context.c
b/drivers/gpu/drm/i915/gem/selftests/mock_context.c
index 8ac6726ec16b..e199d7dbb876 100644
--- a/drivers/gpu/drm/i915/gem/selftests/mock_context.c
+++ b/drivers/gpu/drm/i915/gem/selftests/mock_context.c
@@ -36,7 +36,7 @@ mock_context(struct drm_i915_private *i915,
if (name) {
struct i915_ppgtt *ppgtt;
- strncpy(ctx->name, name, sizeof(ctx->name) - 1);
+ strscpy(ctx->name, name, sizeof(ctx->name));
ppgtt = mock_ppgtt(i915, name);
if (!ppgtt)
---
base-commit: 3669558bdf354cd352be955ef2764cde6a9bf5ec
change-id:
20230914-strncpy-drivers-gpu-drm-i915-gem-selftests-mock_context-c-980c8ecc9142
Best regards,
--
Justin Stitt
[PATCH v2] drm/gma500: refactor deprecated strncpy
`strncpy` is deprecated for use on NUL-terminated destination strings [1]. We should prefer more robust and less ambiguous string interfaces. Since `chan->base.name` is expected to be NUL-terminated, a suitable replacement is `strscpy` [2] due to the fact that it guarantees NUL-termination on the destination buffer without also unnecessarily NUL-padding (since `chan` is kzalloc'd already). Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1] Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2] Link: https://github.com/KSPP/linux/issues/90 Cc: linux-harden...@vger.kernel.org Signed-off-by: Justin Stitt --- Changes in v2: - use sizeof() instead of I2C_NAME_SIZE (thanks Kees, Patrik) - Link to v1: https://lore.kernel.org/r/20230914-drivers-gpu-drm-gma500-oaktrail_lvds_i2c-c-v1-1-0a53a076c...@google.com --- drivers/gpu/drm/gma500/oaktrail_lvds_i2c.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/gma500/oaktrail_lvds_i2c.c b/drivers/gpu/drm/gma500/oaktrail_lvds_i2c.c index 06b5b2d70d48..939c53fd09e8 100644 --- a/drivers/gpu/drm/gma500/oaktrail_lvds_i2c.c +++ b/drivers/gpu/drm/gma500/oaktrail_lvds_i2c.c @@ -141,7 +141,7 @@ struct gma_i2c_chan *oaktrail_lvds_i2c_init(struct drm_device *dev) chan->drm_dev = dev; chan->reg = dev_priv->lpc_gpio_base; - strncpy(chan->base.name, "gma500 LPC", I2C_NAME_SIZE - 1); + strscpy(chan->base.name, "gma500 LPC", sizeof(chan->base.name)); chan->base.owner = THIS_MODULE; chan->base.algo_data = >algo; chan->base.dev.parent = dev->dev; --- base-commit: 3669558bdf354cd352be955ef2764cde6a9bf5ec change-id: 20230914-drivers-gpu-drm-gma500-oaktrail_lvds_i2c-c-a53c6d8bd62f Best regards, -- Justin Stitt
[PATCH v2] drm/etnaviv: refactor deprecated strncpy
`strncpy` is deprecated for use on NUL-terminated destination strings [1]. We should prefer more robust and less ambiguous string interfaces. A suitable replacement is `strscpy_pad` due to the fact that it guarantees NUL-termination on the destination buffer whilst maintaining the NUL-padding behavior that strncpy provides. Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1] Link: https://github.com/KSPP/linux/issues/90 Cc: linux-harden...@vger.kernel.org Cc: Bo YU Signed-off-by: Justin Stitt --- Changes in v2: - use strscpy_pad (thanks Kees) - Link to v1: https://lore.kernel.org/r/20230914-strncpy-drivers-gpu-drm-etnaviv-etnaviv_perfmon-c-v1-1-3adc2d9bf...@google.com --- Similar to [2] which was never picked up. Let's prefer strscpy_pad to strlcpy, though [2]: https://lore.kernel.org/all/20190328080918.9290-1-tsu.y...@gmail.com/ --- drivers/gpu/drm/etnaviv/etnaviv_perfmon.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c index bafdfe49c1d8..dc9dea664a28 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c @@ -511,7 +511,7 @@ int etnaviv_pm_query_dom(struct etnaviv_gpu *gpu, domain->id = domain->iter; domain->nr_signals = dom->nr_signals; - strncpy(domain->name, dom->name, sizeof(domain->name)); + strscpy_pad(domain->name, dom->name, sizeof(domain->name)); domain->iter++; if (domain->iter == nr_domains) @@ -540,7 +540,7 @@ int etnaviv_pm_query_sig(struct etnaviv_gpu *gpu, sig = >signal[signal->iter]; signal->id = signal->iter; - strncpy(signal->name, sig->name, sizeof(signal->name)); + strscpy_pad(signal->name, sig->name, sizeof(signal->name)); signal->iter++; if (signal->iter == dom->nr_signals) --- base-commit: 3669558bdf354cd352be955ef2764cde6a9bf5ec change-id: 20230914-strncpy-drivers-gpu-drm-etnaviv-etnaviv_perfmon-c-dd095491dfde Best regards, -- Justin Stitt
[PATCH] drm/nouveau/pm: refactor deprecated strncpy
`strncpy` is deprecated for use on NUL-terminated destination strings [1].
We should prefer more robust and less ambiguous string interfaces.
A suitable replacement is `strscpy` [2] due to the fact that it guarantees
NUL-termination on the destination buffer without unnecessarily NUL-padding.
Link:
https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings
[1]
Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-harden...@vger.kernel.org
Signed-off-by: Justin Stitt
---
drivers/gpu/drm/nouveau/nvkm/engine/pm/base.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/pm/base.c
b/drivers/gpu/drm/nouveau/nvkm/engine/pm/base.c
index 8fe0444f761e..131db2645f84 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/pm/base.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/pm/base.c
@@ -462,7 +462,7 @@ nvkm_perfmon_mthd_query_domain(struct nvkm_perfmon *perfmon,
args->v0.id = di;
args->v0.signal_nr = nvkm_perfdom_count_perfsig(dom);
- strncpy(args->v0.name, dom->name, sizeof(args->v0.name) - 1);
+ strscpy(args->v0.name, dom->name, sizeof(args->v0.name));
/* Currently only global counters (PCOUNTER) are implemented
* but this will be different for local counters (MP). */
@@ -513,8 +513,7 @@ nvkm_perfmon_mthd_query_signal(struct nvkm_perfmon *perfmon,
snprintf(args->v0.name, sizeof(args->v0.name),
"/%s/%02x", dom->name, si);
} else {
- strncpy(args->v0.name, sig->name,
- sizeof(args->v0.name) - 1);
+ strscpy(args->v0.name, sig->name,
sizeof(args->v0.name));
}
args->v0.signal = si;
@@ -572,7 +571,7 @@ nvkm_perfmon_mthd_query_source(struct nvkm_perfmon *perfmon,
args->v0.source = sig->source[si];
args->v0.mask = src->mask;
- strncpy(args->v0.name, src->name, sizeof(args->v0.name) - 1);
+ strscpy(args->v0.name, src->name, sizeof(args->v0.name));
}
if (++si < source_nr) {
---
base-commit: 3669558bdf354cd352be955ef2764cde6a9bf5ec
change-id:
20230914-strncpy-drivers-gpu-drm-nouveau-nvkm-engine-pm-base-c-38bf9c78bc0f
Best regards,
--
Justin Stitt
[PATCH] drm/nouveau/core: refactor deprecated strncpy
`strncpy` is deprecated for use on NUL-terminated destination strings [1].
We should prefer more robust and less ambiguous string interfaces.
A suitable replacement is `strscpy` [2] due to the fact that it guarantees
NUL-termination on the destination buffer without unnecessarily NUL-padding.
There is likely no bug in the current implementation due to the safeguard:
| cname[sizeof(cname) - 1] = '\0';
... however we can provide simpler and easier to understand code using
the newer (and recommended) `strscpy` api.
Link:
https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings
[1]
Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-harden...@vger.kernel.org
Signed-off-by: Justin Stitt
---
drivers/gpu/drm/nouveau/nvkm/core/firmware.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
b/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
index 91fb494d4009..374212da9e95 100644
--- a/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
+++ b/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
@@ -79,8 +79,7 @@ nvkm_firmware_get(const struct nvkm_subdev *subdev, const
char *fwname, int ver,
int i;
/* Convert device name to lowercase */
- strncpy(cname, device->chip->name, sizeof(cname));
- cname[sizeof(cname) - 1] = '\0';
+ strscpy(cname, device->chip->name, sizeof(cname));
i = strlen(cname);
while (i) {
--i;
---
base-commit: 3669558bdf354cd352be955ef2764cde6a9bf5ec
change-id:
20230914-strncpy-drivers-gpu-drm-nouveau-nvkm-core-firmware-c-791223838b72
Best regards,
--
Justin Stitt
[PATCH] drm/nouveau/nvif: refactor deprecated strncpy
`strncpy` is deprecated and as such we should prefer more robust and
less ambiguous string interfaces.
A suitable replacement is `strscpy_pad` due to the fact that it
guarantees NUL-termination on the destination buffer whilst also
maintaining the NUL-padding behavior that `strncpy` provides. I am not
sure whether NUL-padding is strictly needed but I see in
`nvif_object_ctor()` args is memcpy'd elsewhere so I figured we'd keep
the same functionality.
Link:
https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings
[1]
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-harden...@vger.kernel.org
Signed-off-by: Justin Stitt
---
Note: build-tested only.
---
drivers/gpu/drm/nouveau/nvif/client.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/nouveau/nvif/client.c
b/drivers/gpu/drm/nouveau/nvif/client.c
index a3264a0e933a..3a27245f467f 100644
--- a/drivers/gpu/drm/nouveau/nvif/client.c
+++ b/drivers/gpu/drm/nouveau/nvif/client.c
@@ -69,7 +69,7 @@ nvif_client_ctor(struct nvif_client *parent, const char
*name, u64 device,
} nop = {};
int ret;
- strncpy(args.name, name, sizeof(args.name));
+ strscpy_pad(args.name, name, sizeof(args.name));
ret = nvif_object_ctor(parent != client ? >object : NULL,
name ? name : "nvifClient", 0,
NVIF_CLASS_CLIENT, , sizeof(args),
---
base-commit: 3669558bdf354cd352be955ef2764cde6a9bf5ec
change-id: 20230914-strncpy-drivers-gpu-drm-nouveau-nvif-client-c-82b023c36953
Best regards,
--
Justin Stitt
[PATCH] drm/i915: refactor deprecated strncpy
`strncpy` is deprecated for use on NUL-terminated destination strings [1].
We should prefer more robust and less ambiguous string interfaces.
A suitable replacement is `strscpy` [2] due to the fact that it guarantees
NUL-termination on the destination buffer without unnecessarily NUL-padding.
`ctx` is zero allocated and as such strncpy's NUL-padding behavior was
strictly a performance hit which should now be resolved. It should be
noted, however, that performance is not critical in these selftests,
especially by these margins.
Link:
https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings
[1]
Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-harden...@vger.kernel.org
Signed-off-by: Justin Stitt
---
drivers/gpu/drm/i915/gem/selftests/mock_context.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gem/selftests/mock_context.c
b/drivers/gpu/drm/i915/gem/selftests/mock_context.c
index 8ac6726ec16b..025b9c773b93 100644
--- a/drivers/gpu/drm/i915/gem/selftests/mock_context.c
+++ b/drivers/gpu/drm/i915/gem/selftests/mock_context.c
@@ -36,7 +36,7 @@ mock_context(struct drm_i915_private *i915,
if (name) {
struct i915_ppgtt *ppgtt;
- strncpy(ctx->name, name, sizeof(ctx->name) - 1);
+ strscpy(ctx->name, name, sizeof(ctx->name) - 1);
ppgtt = mock_ppgtt(i915, name);
if (!ppgtt)
---
base-commit: 3669558bdf354cd352be955ef2764cde6a9bf5ec
change-id:
20230914-strncpy-drivers-gpu-drm-i915-gem-selftests-mock_context-c-980c8ecc9142
Best regards,
--
Justin Stitt
[PATCH] drm/gma500: refactor deprecated strncpy
`strncpy` is deprecated for use on NUL-terminated destination strings [1]. We should prefer more robust and less ambiguous string interfaces. Since `chan->base.name` is expected to be NUL-terminated, a suitable replacement is `strscpy` [2] due to the fact that it guarantees NUL-termination on the destination buffer without also unnecessarily NUL-padding. Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1] Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2] Link: https://github.com/KSPP/linux/issues/90 Cc: linux-harden...@vger.kernel.org Signed-off-by: Justin Stitt --- drm/gma500: refactor deprecated strncpy --- drivers/gpu/drm/gma500/oaktrail_lvds_i2c.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/gma500/oaktrail_lvds_i2c.c b/drivers/gpu/drm/gma500/oaktrail_lvds_i2c.c index 06b5b2d70d48..68458cbdd6d5 100644 --- a/drivers/gpu/drm/gma500/oaktrail_lvds_i2c.c +++ b/drivers/gpu/drm/gma500/oaktrail_lvds_i2c.c @@ -141,7 +141,7 @@ struct gma_i2c_chan *oaktrail_lvds_i2c_init(struct drm_device *dev) chan->drm_dev = dev; chan->reg = dev_priv->lpc_gpio_base; - strncpy(chan->base.name, "gma500 LPC", I2C_NAME_SIZE - 1); + strscpy(chan->base.name, "gma500 LPC", I2C_NAME_SIZE - 1); chan->base.owner = THIS_MODULE; chan->base.algo_data = >algo; chan->base.dev.parent = dev->dev; --- base-commit: 3669558bdf354cd352be955ef2764cde6a9bf5ec change-id: 20230914-drivers-gpu-drm-gma500-oaktrail_lvds_i2c-c-a53c6d8bd62f Best regards, -- Justin Stitt
[PATCH] drm/etnaviv: refactor deprecated strncpy
`strncpy` is deprecated for use on NUL-terminated destination strings [1]. We should prefer more robust and less ambiguous string interfaces. A suitable replacement is `strscpy` [2] due to the fact that it guarantees NUL-termination on the destination buffer. Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1] Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2] Link: https://github.com/KSPP/linux/issues/90 Cc: linux-harden...@vger.kernel.org Cc: Bo YU Signed-off-by: Justin Stitt --- Similar to [1] which was never picked up. Let's prefer strscpy to strlcpy, though [1]: https://lore.kernel.org/all/20190328080918.9290-1-tsu.y...@gmail.com/ --- drivers/gpu/drm/etnaviv/etnaviv_perfmon.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c index bafdfe49c1d8..9e7bebcf24eb 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c @@ -511,7 +511,7 @@ int etnaviv_pm_query_dom(struct etnaviv_gpu *gpu, domain->id = domain->iter; domain->nr_signals = dom->nr_signals; - strncpy(domain->name, dom->name, sizeof(domain->name)); + strscpy(domain->name, dom->name, sizeof(domain->name)); domain->iter++; if (domain->iter == nr_domains) @@ -540,7 +540,7 @@ int etnaviv_pm_query_sig(struct etnaviv_gpu *gpu, sig = >signal[signal->iter]; signal->id = signal->iter; - strncpy(signal->name, sig->name, sizeof(signal->name)); + strscpy(signal->name, sig->name, sizeof(signal->name)); signal->iter++; if (signal->iter == dom->nr_signals) --- base-commit: 3669558bdf354cd352be955ef2764cde6a9bf5ec change-id: 20230914-strncpy-drivers-gpu-drm-etnaviv-etnaviv_perfmon-c-dd095491dfde Best regards, -- Justin Stitt
[PATCH] drm/modes: refactor deprecated strncpy
`strncpy` is deprecated for use on NUL-terminated destination strings [1].
We should prefer more robust and less ambiguous string interfaces.
A suitable replacement is `strscpy` [2] due to the fact that it guarantees
NUL-termination on the destination buffer and doesn't incur the
performance loss of unnecessarily NUL-padding.
Link:
https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings
[1]
Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-harden...@vger.kernel.org
Cc: Xu Panda
Signed-off-by: Justin Stitt
---
This patch is basically a resend of [1] by Xu but rebased onto mainline.
This patch is also similar to:
commit 0f9aa074c92dd ("drm/modes: Use strscpy() to copy command-line mode name")
[1]: https://lore.kernel.org/all/202212051935289159...@zte.com.cn/
---
drivers/gpu/drm/drm_modes.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index ac9a406250c5..c702a8b866cf 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -2617,8 +2617,7 @@ void drm_mode_convert_to_umode(struct drm_mode_modeinfo
*out,
break;
}
- strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN);
- out->name[DRM_DISPLAY_MODE_LEN-1] = 0;
+ strscpy(out->name, in->name, DRM_DISPLAY_MODE_LEN);
}
/**
@@ -2659,8 +2658,7 @@ int drm_mode_convert_umode(struct drm_device *dev,
* useful for the kernel->userspace direction anyway.
*/
out->type = in->type & DRM_MODE_TYPE_ALL;
- strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN);
- out->name[DRM_DISPLAY_MODE_LEN-1] = 0;
+ strscpy(out->name, in->name, DRM_DISPLAY_MODE_LEN);
/* Clearing picture aspect ratio bits from out flags,
* as the aspect-ratio information is not stored in
---
base-commit: 3669558bdf354cd352be955ef2764cde6a9bf5ec
change-id: 20230914-strncpy-drivers-gpu-drm-drm_modes-c-a35d782cad01
Best regards,
--
Justin Stitt
Re: [PATCH] habanalabs/gaudi: refactor deprecated strncpy
This patch as well as two other related patches were combined into a single patch [1] On Thu, Aug 24, 2023 at 1:41 PM Justin Stitt wrote: > > `strncpy` is deprecated for use on NUL-terminated destination strings [1]. > > A suitable replacement is `strscpy` [2] due to the fact that it > guarantees NUL-termination on its destination buffer argument which is > _not_ the case for `strncpy`! > > Link: > www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings[1] > Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html > [2] > Link: https://github.com/KSPP/linux/issues/90 > Cc: linux-harden...@vger.kernel.org > Signed-off-by: Justin Stitt > --- > Note: build-tested only > --- > drivers/accel/habanalabs/gaudi/gaudi.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/accel/habanalabs/gaudi/gaudi.c > b/drivers/accel/habanalabs/gaudi/gaudi.c > index 056e2ef44afb..f175456cdef0 100644 > --- a/drivers/accel/habanalabs/gaudi/gaudi.c > +++ b/drivers/accel/habanalabs/gaudi/gaudi.c > @@ -660,7 +660,7 @@ static int gaudi_set_fixed_properties(struct hl_device > *hdev) > prop->pcie_dbi_base_address = mmPCIE_DBI_BASE; > prop->pcie_aux_dbi_reg_addr = CFG_BASE + mmPCIE_AUX_DBI; > > - strncpy(prop->cpucp_info.card_name, GAUDI_DEFAULT_CARD_NAME, > + strscpy(prop->cpucp_info.card_name, GAUDI_DEFAULT_CARD_NAME, > CARD_NAME_MAX_LEN); > > prop->max_pending_cs = GAUDI_MAX_PENDING_CS; > @@ -8000,7 +8000,7 @@ static int gaudi_cpucp_info_get(struct hl_device *hdev) > return rc; > > if (!strlen(prop->cpucp_info.card_name)) > - strncpy(prop->cpucp_info.card_name, GAUDI_DEFAULT_CARD_NAME, > + strscpy(prop->cpucp_info.card_name, GAUDI_DEFAULT_CARD_NAME, > CARD_NAME_MAX_LEN); > > hdev->card_type = le32_to_cpu(hdev->asic_prop.cpucp_info.card_type); > > --- > base-commit: f9604036a3fb6149badf346994b46b03f9292db7 > change-id: > 20230824-strncpy-drivers-accel-habanalabs-gaudi-gaudi-c-f0b5814ced38 > > Best regards, > -- > Justin Stitt > [1]: https://lore.kernel.org/r/20230825-strncpy-habanalabs-combined-v1-1-daa05a89b...@google.com
Re: [PATCH] habanalabs/gaudi2: refactor deprecated strncpy
This patch as well as two other related patches were combined into a single patch [1] On Thu, Aug 24, 2023 at 1:45 PM Justin Stitt wrote: > > `strncpy` is deprecated for use on NUL-terminated destination strings [1]. > > A suitable replacement is `strscpy` [2] due to the fact that it > guarantees NUL-termination on its destination buffer argument which is > _not_ the case for `strncpy`! > > Link: > www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings[1] > Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html > [2] > Link: https://github.com/KSPP/linux/issues/90 > Cc: linux-harden...@vger.kernel.org > Signed-off-by: Justin Stitt > --- > Note: build-tested only. > --- > drivers/accel/habanalabs/gaudi2/gaudi2.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/accel/habanalabs/gaudi2/gaudi2.c > b/drivers/accel/habanalabs/gaudi2/gaudi2.c > index 20c4583f12b0..755b2d92357d 100644 > --- a/drivers/accel/habanalabs/gaudi2/gaudi2.c > +++ b/drivers/accel/habanalabs/gaudi2/gaudi2.c > @@ -2431,7 +2431,7 @@ static int gaudi2_set_fixed_properties(struct hl_device > *hdev) > prop->pcie_dbi_base_address = CFG_BASE + mmPCIE_DBI_BASE; > prop->pcie_aux_dbi_reg_addr = CFG_BASE + mmPCIE_AUX_DBI; > > - strncpy(prop->cpucp_info.card_name, GAUDI2_DEFAULT_CARD_NAME, > CARD_NAME_MAX_LEN); > + strscpy(prop->cpucp_info.card_name, GAUDI2_DEFAULT_CARD_NAME, > CARD_NAME_MAX_LEN); > > prop->mme_master_slave_mode = 1; > > @@ -2884,7 +2884,7 @@ static int gaudi2_cpucp_info_get(struct hl_device *hdev) > } > > if (!strlen(prop->cpucp_info.card_name)) > - strncpy(prop->cpucp_info.card_name, GAUDI2_DEFAULT_CARD_NAME, > CARD_NAME_MAX_LEN); > + strscpy(prop->cpucp_info.card_name, GAUDI2_DEFAULT_CARD_NAME, > CARD_NAME_MAX_LEN); > > /* Overwrite binning masks with the actual binning values from F/W */ > hdev->dram_binning = prop->cpucp_info.dram_binning_mask; > > --- > base-commit: f9604036a3fb6149badf346994b46b03f9292db7 > change-id: > 20230824-strncpy-drivers-accel-habanalabs-gaudi2-gaudi2-c-0b3f717bee12 > > Best regards, > -- > Justin Stitt > [1]: https://lore.kernel.org/r/20230825-strncpy-habanalabs-combined-v1-1-daa05a89b...@google.com
Re: [PATCH] habanalabs/goya: refactor deprecated strncpy
This patch as well as two other related patches were combined into a single patch [1] Thu, Aug 24, 2023 at 1:49 PM Justin Stitt wrote: > > `strncpy` is deprecated for use on NUL-terminated destination strings [1]. > > A suitable replacement is `strscpy` [2] due to the fact that it > guarantees NUL-termination on its destination buffer argument which is > _not_ the case for `strncpy`! > > Link: > www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings[1] > Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html > [2] > Link: https://github.com/KSPP/linux/issues/90 > Cc: linux-harden...@vger.kernel.org > Signed-off-by: Justin Stitt > --- > Note: build-tested only. > --- > drivers/accel/habanalabs/goya/goya.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/accel/habanalabs/goya/goya.c > b/drivers/accel/habanalabs/goya/goya.c > index 7c685e6075f6..d0ac7065f3d7 100644 > --- a/drivers/accel/habanalabs/goya/goya.c > +++ b/drivers/accel/habanalabs/goya/goya.c > @@ -466,7 +466,7 @@ int goya_set_fixed_properties(struct hl_device *hdev) > prop->pcie_dbi_base_address = mmPCIE_DBI_BASE; > prop->pcie_aux_dbi_reg_addr = CFG_BASE + mmPCIE_AUX_DBI; > > - strncpy(prop->cpucp_info.card_name, GOYA_DEFAULT_CARD_NAME, > + strscpy(prop->cpucp_info.card_name, GOYA_DEFAULT_CARD_NAME, > CARD_NAME_MAX_LEN); > > prop->max_pending_cs = GOYA_MAX_PENDING_CS; > @@ -5122,7 +5122,7 @@ int goya_cpucp_info_get(struct hl_device *hdev) > } > > if (!strlen(prop->cpucp_info.card_name)) > - strncpy(prop->cpucp_info.card_name, GOYA_DEFAULT_CARD_NAME, > + strscpy(prop->cpucp_info.card_name, GOYA_DEFAULT_CARD_NAME, > CARD_NAME_MAX_LEN); > > return 0; > > --- > base-commit: f9604036a3fb6149badf346994b46b03f9292db7 > change-id: 20230824-strncpy-drivers-accel-habanalabs-goya-goya-c-2a05a2202c78 > > Best regards, > -- > Justin Stitt > [1]: https://lore.kernel.org/r/20230825-strncpy-habanalabs-combined-v1-1-daa05a89b...@google.com
[PATCH] accel/habanalabs: refactor deprecated strncpy to strscpy_pad
`strncpy` is deprecated for use on NUL-terminated destination strings [1]. We see that `prop->cpucp_info.card_name` is supposed to be NUL-terminated based on its usage within `__hwmon_device_register()` (wherein it's called "name"): | if (name && (!strlen(name) || strpbrk(name, "-* \t\n"))) | dev_warn(dev, |"hwmon: '%s' is not a valid name attribute, please fix\n", |name); A suitable replacement is `strscpy_pad` [2] due to the fact that it guarantees both NUL-termination and NUL-padding on its destination buffer. NUL-padding on `prop->cpucp_info.card_name` is not strictly necessary as `hdev->prop` is explicitly zero-initialized but should be used regardless as it gets copied out to userspace directly -- as per Kees' suggestion. Link: www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings[1] Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2] Link: https://github.com/KSPP/linux/issues/90 Cc: linux-harden...@vger.kernel.org Signed-off-by: Justin Stitt Suggested-by: Kees Cook --- Note: build-tested only This patch combines three previous strncpy refactor patches into one. 1) https://lore.kernel.org/all/20230824-strncpy-drivers-accel-habanalabs-gaudi-gaudi-c-v1-1-a7fb90547...@google.com/ 2) https://lore.kernel.org/all/20230824-strncpy-drivers-accel-habanalabs-gaudi2-gaudi2-c-v1-1-1a37b6557...@google.com/ 3) https://lore.kernel.org/all/20230824-strncpy-drivers-accel-habanalabs-goya-goya-c-v1-1-b81d5639e...@google.com/ --- drivers/accel/habanalabs/gaudi/gaudi.c | 4 ++-- drivers/accel/habanalabs/gaudi2/gaudi2.c | 4 ++-- drivers/accel/habanalabs/goya/goya.c | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/accel/habanalabs/gaudi/gaudi.c b/drivers/accel/habanalabs/gaudi/gaudi.c index 056e2ef44afb..1b5fe4d0cf5d 100644 --- a/drivers/accel/habanalabs/gaudi/gaudi.c +++ b/drivers/accel/habanalabs/gaudi/gaudi.c @@ -660,7 +660,7 @@ static int gaudi_set_fixed_properties(struct hl_device *hdev) prop->pcie_dbi_base_address = mmPCIE_DBI_BASE; prop->pcie_aux_dbi_reg_addr = CFG_BASE + mmPCIE_AUX_DBI; - strncpy(prop->cpucp_info.card_name, GAUDI_DEFAULT_CARD_NAME, + strscpy_pad(prop->cpucp_info.card_name, GAUDI_DEFAULT_CARD_NAME, CARD_NAME_MAX_LEN); prop->max_pending_cs = GAUDI_MAX_PENDING_CS; @@ -8000,7 +8000,7 @@ static int gaudi_cpucp_info_get(struct hl_device *hdev) return rc; if (!strlen(prop->cpucp_info.card_name)) - strncpy(prop->cpucp_info.card_name, GAUDI_DEFAULT_CARD_NAME, + strscpy_pad(prop->cpucp_info.card_name, GAUDI_DEFAULT_CARD_NAME, CARD_NAME_MAX_LEN); hdev->card_type = le32_to_cpu(hdev->asic_prop.cpucp_info.card_type); diff --git a/drivers/accel/habanalabs/gaudi2/gaudi2.c b/drivers/accel/habanalabs/gaudi2/gaudi2.c index 20c4583f12b0..2ba7a50103bc 100644 --- a/drivers/accel/habanalabs/gaudi2/gaudi2.c +++ b/drivers/accel/habanalabs/gaudi2/gaudi2.c @@ -2431,7 +2431,7 @@ static int gaudi2_set_fixed_properties(struct hl_device *hdev) prop->pcie_dbi_base_address = CFG_BASE + mmPCIE_DBI_BASE; prop->pcie_aux_dbi_reg_addr = CFG_BASE + mmPCIE_AUX_DBI; - strncpy(prop->cpucp_info.card_name, GAUDI2_DEFAULT_CARD_NAME, CARD_NAME_MAX_LEN); + strscpy_pad(prop->cpucp_info.card_name, GAUDI2_DEFAULT_CARD_NAME, CARD_NAME_MAX_LEN); prop->mme_master_slave_mode = 1; @@ -2884,7 +2884,7 @@ static int gaudi2_cpucp_info_get(struct hl_device *hdev) } if (!strlen(prop->cpucp_info.card_name)) - strncpy(prop->cpucp_info.card_name, GAUDI2_DEFAULT_CARD_NAME, CARD_NAME_MAX_LEN); + strscpy_pad(prop->cpucp_info.card_name, GAUDI2_DEFAULT_CARD_NAME, CARD_NAME_MAX_LEN); /* Overwrite binning masks with the actual binning values from F/W */ hdev->dram_binning = prop->cpucp_info.dram_binning_mask; diff --git a/drivers/accel/habanalabs/goya/goya.c b/drivers/accel/habanalabs/goya/goya.c index 7c685e6075f6..024ccf2e159b 100644 --- a/drivers/accel/habanalabs/goya/goya.c +++ b/drivers/accel/habanalabs/goya/goya.c @@ -466,7 +466,7 @@ int goya_set_fixed_properties(struct hl_device *hdev) prop->pcie_dbi_base_address = mmPCIE_DBI_BASE; prop->pcie_aux_dbi_reg_addr = CFG_BASE + mmPCIE_AUX_DBI; - strncpy(prop->cpucp_info.card_name, GOYA_DEFAULT_CARD_NAME, + strscpy_pad(prop->cpucp_info.card_name, GOYA_DEFAULT_CARD_NAME, CARD_NAME_MAX_LEN); prop->max_pending_cs = GOYA_MAX_PENDING_CS; @@ -5122,7 +5122,7 @@ int goya_cpucp_info_get(struct hl_device *hdev) } if (!strlen(prop->cpucp_info.card_name)) -
[PATCH] accel/ivpu: refactor deprecated strncpy
`strncpy` is deprecated for use on NUL-terminated destination strings [1]. A suitable replacement is `strscpy` [2] due to the fact that it guarantees NUL-termination on its destination buffer argument which is _not_ the case for `strncpy`! Also remove extraneous if-statement as it can never be entered. The return value from `strncpy` is it's first argument. In this case, `...dyndbg_cmd` is an array: | char dyndbg_cmd[VPU_DYNDBG_CMD_MAX_LEN]; ^^ This can never be NULL which means `strncpy`'s return value cannot be NULL here. Just use `strscpy` which is more robust and results in simpler and less ambiguous code. Moreover, remove needless `... - 1` as `strscpy`'s implementation ensures NUL-termination and we do not need to carefully dance around ending boundaries with a "- 1" anymore. Link: www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings[1] Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2] Link: https://github.com/KSPP/linux/issues/90 Cc: linux-harden...@vger.kernel.org Signed-off-by: Justin Stitt --- Note: build-tested only. --- drivers/accel/ivpu/ivpu_jsm_msg.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/accel/ivpu/ivpu_jsm_msg.c b/drivers/accel/ivpu/ivpu_jsm_msg.c index 831bfd2b2d39..bdddef2c59ee 100644 --- a/drivers/accel/ivpu/ivpu_jsm_msg.c +++ b/drivers/accel/ivpu/ivpu_jsm_msg.c @@ -118,8 +118,7 @@ int ivpu_jsm_dyndbg_control(struct ivpu_device *vdev, char *command, size_t size struct vpu_jsm_msg resp; int ret; - if (!strncpy(req.payload.dyndbg_control.dyndbg_cmd, command, VPU_DYNDBG_CMD_MAX_LEN - 1)) - return -ENOMEM; + strscpy(req.payload.dyndbg_control.dyndbg_cmd, command, VPU_DYNDBG_CMD_MAX_LEN); ret = ivpu_ipc_send_receive(vdev, , VPU_JSM_MSG_DYNDBG_CONTROL_RSP, , VPU_IPC_CHAN_ASYNC_CMD, vdev->timeout.jsm); --- base-commit: f9604036a3fb6149badf346994b46b03f9292db7 change-id: 20230824-strncpy-drivers-accel-ivpu-ivpu_jsm_msg-c-2422f7f00fc8 Best regards, -- Justin Stitt
[PATCH] habanalabs/goya: refactor deprecated strncpy
`strncpy` is deprecated for use on NUL-terminated destination strings [1]. A suitable replacement is `strscpy` [2] due to the fact that it guarantees NUL-termination on its destination buffer argument which is _not_ the case for `strncpy`! Link: www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings[1] Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2] Link: https://github.com/KSPP/linux/issues/90 Cc: linux-harden...@vger.kernel.org Signed-off-by: Justin Stitt --- Note: build-tested only. --- drivers/accel/habanalabs/goya/goya.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/accel/habanalabs/goya/goya.c b/drivers/accel/habanalabs/goya/goya.c index 7c685e6075f6..d0ac7065f3d7 100644 --- a/drivers/accel/habanalabs/goya/goya.c +++ b/drivers/accel/habanalabs/goya/goya.c @@ -466,7 +466,7 @@ int goya_set_fixed_properties(struct hl_device *hdev) prop->pcie_dbi_base_address = mmPCIE_DBI_BASE; prop->pcie_aux_dbi_reg_addr = CFG_BASE + mmPCIE_AUX_DBI; - strncpy(prop->cpucp_info.card_name, GOYA_DEFAULT_CARD_NAME, + strscpy(prop->cpucp_info.card_name, GOYA_DEFAULT_CARD_NAME, CARD_NAME_MAX_LEN); prop->max_pending_cs = GOYA_MAX_PENDING_CS; @@ -5122,7 +5122,7 @@ int goya_cpucp_info_get(struct hl_device *hdev) } if (!strlen(prop->cpucp_info.card_name)) - strncpy(prop->cpucp_info.card_name, GOYA_DEFAULT_CARD_NAME, + strscpy(prop->cpucp_info.card_name, GOYA_DEFAULT_CARD_NAME, CARD_NAME_MAX_LEN); return 0; --- base-commit: f9604036a3fb6149badf346994b46b03f9292db7 change-id: 20230824-strncpy-drivers-accel-habanalabs-goya-goya-c-2a05a2202c78 Best regards, -- Justin Stitt
[PATCH] habanalabs/gaudi2: refactor deprecated strncpy
`strncpy` is deprecated for use on NUL-terminated destination strings [1]. A suitable replacement is `strscpy` [2] due to the fact that it guarantees NUL-termination on its destination buffer argument which is _not_ the case for `strncpy`! Link: www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings[1] Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2] Link: https://github.com/KSPP/linux/issues/90 Cc: linux-harden...@vger.kernel.org Signed-off-by: Justin Stitt --- Note: build-tested only. --- drivers/accel/habanalabs/gaudi2/gaudi2.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/accel/habanalabs/gaudi2/gaudi2.c b/drivers/accel/habanalabs/gaudi2/gaudi2.c index 20c4583f12b0..755b2d92357d 100644 --- a/drivers/accel/habanalabs/gaudi2/gaudi2.c +++ b/drivers/accel/habanalabs/gaudi2/gaudi2.c @@ -2431,7 +2431,7 @@ static int gaudi2_set_fixed_properties(struct hl_device *hdev) prop->pcie_dbi_base_address = CFG_BASE + mmPCIE_DBI_BASE; prop->pcie_aux_dbi_reg_addr = CFG_BASE + mmPCIE_AUX_DBI; - strncpy(prop->cpucp_info.card_name, GAUDI2_DEFAULT_CARD_NAME, CARD_NAME_MAX_LEN); + strscpy(prop->cpucp_info.card_name, GAUDI2_DEFAULT_CARD_NAME, CARD_NAME_MAX_LEN); prop->mme_master_slave_mode = 1; @@ -2884,7 +2884,7 @@ static int gaudi2_cpucp_info_get(struct hl_device *hdev) } if (!strlen(prop->cpucp_info.card_name)) - strncpy(prop->cpucp_info.card_name, GAUDI2_DEFAULT_CARD_NAME, CARD_NAME_MAX_LEN); + strscpy(prop->cpucp_info.card_name, GAUDI2_DEFAULT_CARD_NAME, CARD_NAME_MAX_LEN); /* Overwrite binning masks with the actual binning values from F/W */ hdev->dram_binning = prop->cpucp_info.dram_binning_mask; --- base-commit: f9604036a3fb6149badf346994b46b03f9292db7 change-id: 20230824-strncpy-drivers-accel-habanalabs-gaudi2-gaudi2-c-0b3f717bee12 Best regards, -- Justin Stitt
[PATCH] habanalabs/gaudi: refactor deprecated strncpy
`strncpy` is deprecated for use on NUL-terminated destination strings [1]. A suitable replacement is `strscpy` [2] due to the fact that it guarantees NUL-termination on its destination buffer argument which is _not_ the case for `strncpy`! Link: www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings[1] Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2] Link: https://github.com/KSPP/linux/issues/90 Cc: linux-harden...@vger.kernel.org Signed-off-by: Justin Stitt --- Note: build-tested only --- drivers/accel/habanalabs/gaudi/gaudi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/accel/habanalabs/gaudi/gaudi.c b/drivers/accel/habanalabs/gaudi/gaudi.c index 056e2ef44afb..f175456cdef0 100644 --- a/drivers/accel/habanalabs/gaudi/gaudi.c +++ b/drivers/accel/habanalabs/gaudi/gaudi.c @@ -660,7 +660,7 @@ static int gaudi_set_fixed_properties(struct hl_device *hdev) prop->pcie_dbi_base_address = mmPCIE_DBI_BASE; prop->pcie_aux_dbi_reg_addr = CFG_BASE + mmPCIE_AUX_DBI; - strncpy(prop->cpucp_info.card_name, GAUDI_DEFAULT_CARD_NAME, + strscpy(prop->cpucp_info.card_name, GAUDI_DEFAULT_CARD_NAME, CARD_NAME_MAX_LEN); prop->max_pending_cs = GAUDI_MAX_PENDING_CS; @@ -8000,7 +8000,7 @@ static int gaudi_cpucp_info_get(struct hl_device *hdev) return rc; if (!strlen(prop->cpucp_info.card_name)) - strncpy(prop->cpucp_info.card_name, GAUDI_DEFAULT_CARD_NAME, + strscpy(prop->cpucp_info.card_name, GAUDI_DEFAULT_CARD_NAME, CARD_NAME_MAX_LEN); hdev->card_type = le32_to_cpu(hdev->asic_prop.cpucp_info.card_type); --- base-commit: f9604036a3fb6149badf346994b46b03f9292db7 change-id: 20230824-strncpy-drivers-accel-habanalabs-gaudi-gaudi-c-f0b5814ced38 Best regards, -- Justin Stitt
[PATCH] accel/habanalabs: refactor deprecated strncpy
`strncpy` is deprecated for use on NUL-terminated destination strings [1]. A suitable replacement is `strscpy` [2] due to the fact that it guarantees NUL-termination on its destination buffer argument which is _not_ the case for `strncpy`! There is likely no bug happening in this case since HL_STR_MAX is strictly larger than all source strings. Nonetheless, prefer a safer and more robust interface. It should also be noted that `strscpy` will not pad like `strncpy`. If this NUL-padding behavior is _required_ we should use `strscpy_pad` instead of `strscpy`. Link: www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings[1] Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2] Link: https://github.com/KSPP/linux/issues/90 Cc: linux-harden...@vger.kernel.org Signed-off-by: Justin Stitt --- Note: build-tested only. --- drivers/accel/habanalabs/common/habanalabs_drv.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/accel/habanalabs/common/habanalabs_drv.c b/drivers/accel/habanalabs/common/habanalabs_drv.c index 7263e84c1a4d..d9a3418b5ae4 100644 --- a/drivers/accel/habanalabs/common/habanalabs_drv.c +++ b/drivers/accel/habanalabs/common/habanalabs_drv.c @@ -408,13 +408,13 @@ static int create_hdev(struct hl_device **dev, struct pci_dev *pdev) hdev->pdev = pdev; /* Assign status description string */ - strncpy(hdev->status[HL_DEVICE_STATUS_OPERATIONAL], "operational", HL_STR_MAX); - strncpy(hdev->status[HL_DEVICE_STATUS_IN_RESET], "in reset", HL_STR_MAX); - strncpy(hdev->status[HL_DEVICE_STATUS_MALFUNCTION], "disabled", HL_STR_MAX); - strncpy(hdev->status[HL_DEVICE_STATUS_NEEDS_RESET], "needs reset", HL_STR_MAX); - strncpy(hdev->status[HL_DEVICE_STATUS_IN_DEVICE_CREATION], - "in device creation", HL_STR_MAX); - strncpy(hdev->status[HL_DEVICE_STATUS_IN_RESET_AFTER_DEVICE_RELEASE], + strscpy(hdev->status[HL_DEVICE_STATUS_OPERATIONAL], "operational", HL_STR_MAX); + strscpy(hdev->status[HL_DEVICE_STATUS_IN_RESET], "in reset", HL_STR_MAX); + strscpy(hdev->status[HL_DEVICE_STATUS_MALFUNCTION], "disabled", HL_STR_MAX); + strscpy(hdev->status[HL_DEVICE_STATUS_NEEDS_RESET], "needs reset", HL_STR_MAX); + strscpy(hdev->status[HL_DEVICE_STATUS_IN_DEVICE_CREATION], + "in device creation", HL_STR_MAX); + strscpy(hdev->status[HL_DEVICE_STATUS_IN_RESET_AFTER_DEVICE_RELEASE], "in reset after device release", HL_STR_MAX); --- base-commit: 706a741595047797872e669b3101429ab8d378ef change-id: 20230823-strncpy-drivers-accel-habanalabs-common-habanalabs_drv-7ffecf6882ed Best regards, -- Justin Stitt
[PATCH] drm: pl111: fix clang -Wvoid-pointer-to-enum-cast warning
When building with clang 18 I see the following warnings:
|1. drivers/gpu/drm/pl111/pl111_versatile.c:487:24: warning: cast to smaller
| integer type 'enum versatile_clcd' from 'const void *'
[-Wvoid-pointer-to-enum-cast]
| 487 | versatile_clcd_type = (enum
versatile_clcd)clcd_id->data;
-
|2. drivers/gpu/drm/pl111/pl111_versatile.c:507:26: warning: cast to smaller
| integer type 'enum versatile_clcd' from 'const void *'
[-Wvoid-pointer-to-enum-cast]
| 507 | versatile_clcd_type = (enum
versatile_clcd)clcd_id->data;
This is due to the fact that `clcd_id->data` is a void* while `enum
versatile_clcd`
has the size of an int.
Cast `clcd_id->data` to a uintptr_t to silence the above warning for
clang builds using W=1
Link: https://github.com/ClangBuiltLinux/linux/issues/1910
Reported-by: Nathan Chancellor
Signed-off-by: Justin Stitt
---
drivers/gpu/drm/pl111/pl111_versatile.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/pl111/pl111_versatile.c
b/drivers/gpu/drm/pl111/pl111_versatile.c
index 00c3ebd32359..d6fd9e51377e 100644
--- a/drivers/gpu/drm/pl111/pl111_versatile.c
+++ b/drivers/gpu/drm/pl111/pl111_versatile.c
@@ -484,7 +484,7 @@ int pl111_versatile_init(struct device *dev, struct
pl111_drm_dev_private *priv)
return 0;
}
- versatile_clcd_type = (enum versatile_clcd)clcd_id->data;
+ versatile_clcd_type = (enum versatile_clcd)(uintptr_t)clcd_id->data;
/* Versatile Express special handling */
if (versatile_clcd_type == VEXPRESS_CLCD_V2M) {
@@ -504,7 +504,7 @@ int pl111_versatile_init(struct device *dev, struct
pl111_drm_dev_private *priv)
np = of_find_matching_node_and_match(NULL, impd1_clcd_of_match,
_id);
if (np)
- versatile_clcd_type = (enum
versatile_clcd)clcd_id->data;
+ versatile_clcd_type = (enum
versatile_clcd)(uintptr_t)clcd_id->data;
}
map = syscon_node_to_regmap(np);
---
base-commit: 2ccdd1b13c591d306f0401d98dedc4bdcd02b421
change-id: 20230816-void-drivers-gpu-drm-pl111-pl111_versatile-43b109cfa7ad
Best regards,
--
Justin Stitt
[PATCH] drm/repaper: fix -Wvoid-pointer-to-enum-cast warning
When building with clang 18 I see the following warning:
| drivers/gpu/drm/tiny/repaper.c:952:11: warning: cast to smaller integer
| type 'enum repaper_model' from 'const void *'
[-Wvoid-pointer-to-enum-cast]
| 952 | model = (enum repaper_model)match;
|
This is due to the fact that `match` is a void* while `enum repaper_model`
has the size of an int.
Add uintptr_t cast to silence clang warning while also keeping enum cast
for readability and consistency with other `model` assignment just a
few lines below:
| model = (enum repaper_model)spi_id->driver_data;
Link: https://github.com/ClangBuiltLinux/linux/issues/1910
Reported-by: Nathan Chancellor
Signed-off-by: Justin Stitt
---
drivers/gpu/drm/tiny/repaper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/repaper.c
index c2677d081a7b..165f2099e7d8 100644
--- a/drivers/gpu/drm/tiny/repaper.c
+++ b/drivers/gpu/drm/tiny/repaper.c
@@ -949,7 +949,7 @@ static int repaper_probe(struct spi_device *spi)
match = device_get_match_data(dev);
if (match) {
- model = (enum repaper_model)match;
+ model = (enum repaper_model)(uintptr_t)match;
} else {
spi_id = spi_get_device_id(spi);
model = (enum repaper_model)spi_id->driver_data;
---
base-commit: 2ccdd1b13c591d306f0401d98dedc4bdcd02b421
change-id: 20230816-void-drivers-gpu-drm-tiny-repaper-a08321cd99d7
Best regards,
--
Justin Stitt
