2023-07-03 17:32:31

by Azeem Shaikh

[permalink] [raw]
Subject: [PATCH 0/2] kyber, blk-wbt: Replace strlcpy with strscpy

This patch series replaces strlcpy in the kyber and blk-wbt tracing subsystems wherever trivial
replacement is possible, i.e return value from strlcpy is unused. The patches
themselves are independent of each other and are applied to different subsystems. They are
included as a series for ease of review.

Note to reviewers: MAINTAINERS file does not specify clear ownership of
these files so I have addressed these to the latest committer for these
files: Jens Axboe <[email protected]>.

Azeem Shaikh (2):
kyber: Replace strlcpy with strscpy
blk-wbt: Replace strlcpy with strscpy

include/trace/events/kyber.h | 8 ++++----
include/trace/events/wbt.h | 8 ++++----
2 files changed, 8 insertions(+), 8 deletions(-)

--
2.41.0.255.g8b1d071c50-goog



2023-07-03 17:48:39

by Azeem Shaikh

[permalink] [raw]
Subject: [PATCH 1/2] kyber: Replace strlcpy with strscpy

strlcpy() reads the entire source buffer first.
This read may exceed the destination size limit.
This is both inefficient and can lead to linear read
overflows if a source string is not NUL-terminated [1].
In an effort to remove strlcpy() completely [2], replace
strlcpy() here with strscpy().
No return values were used, so direct replacement is safe.

[1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy
[2] https://github.com/KSPP/linux/issues/89

Signed-off-by: Azeem Shaikh <[email protected]>
---
include/trace/events/kyber.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/trace/events/kyber.h b/include/trace/events/kyber.h
index bf7533f171ff..9d44781efc1c 100644
--- a/include/trace/events/kyber.h
+++ b/include/trace/events/kyber.h
@@ -31,8 +31,8 @@ TRACE_EVENT(kyber_latency,

TP_fast_assign(
__entry->dev = dev;
- strlcpy(__entry->domain, domain, sizeof(__entry->domain));
- strlcpy(__entry->type, type, sizeof(__entry->type));
+ strscpy(__entry->domain, domain, sizeof(__entry->domain));
+ strscpy(__entry->type, type, sizeof(__entry->type));
__entry->percentile = percentile;
__entry->numerator = numerator;
__entry->denominator = denominator;
@@ -59,7 +59,7 @@ TRACE_EVENT(kyber_adjust,

TP_fast_assign(
__entry->dev = dev;
- strlcpy(__entry->domain, domain, sizeof(__entry->domain));
+ strscpy(__entry->domain, domain, sizeof(__entry->domain));
__entry->depth = depth;
),

@@ -81,7 +81,7 @@ TRACE_EVENT(kyber_throttled,

TP_fast_assign(
__entry->dev = dev;
- strlcpy(__entry->domain, domain, sizeof(__entry->domain));
+ strscpy(__entry->domain, domain, sizeof(__entry->domain));
),

TP_printk("%d,%d %s", MAJOR(__entry->dev), MINOR(__entry->dev),
--
2.41.0.255.g8b1d071c50-goog


2023-07-03 17:49:38

by Azeem Shaikh

[permalink] [raw]
Subject: [PATCH 2/2] blk-wbt: Replace strlcpy with strscpy

strlcpy() reads the entire source buffer first.
This read may exceed the destination size limit.
This is both inefficient and can lead to linear read
overflows if a source string is not NUL-terminated [1].
In an effort to remove strlcpy() completely [2], replace
strlcpy() here with strscpy().
No return values were used, so direct replacement is safe.

[1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy
[2] https://github.com/KSPP/linux/issues/89

Signed-off-by: Azeem Shaikh <[email protected]>
---
include/trace/events/wbt.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/trace/events/wbt.h b/include/trace/events/wbt.h
index 9c66e59d859c..4661f0d27062 100644
--- a/include/trace/events/wbt.h
+++ b/include/trace/events/wbt.h
@@ -33,7 +33,7 @@ TRACE_EVENT(wbt_stat,
),

TP_fast_assign(
- strlcpy(__entry->name, bdi_dev_name(bdi),
+ strscpy(__entry->name, bdi_dev_name(bdi),
ARRAY_SIZE(__entry->name));
__entry->rmean = stat[0].mean;
__entry->rmin = stat[0].min;
@@ -68,7 +68,7 @@ TRACE_EVENT(wbt_lat,
),

TP_fast_assign(
- strlcpy(__entry->name, bdi_dev_name(bdi),
+ strscpy(__entry->name, bdi_dev_name(bdi),
ARRAY_SIZE(__entry->name));
__entry->lat = div_u64(lat, 1000);
),
@@ -105,7 +105,7 @@ TRACE_EVENT(wbt_step,
),

TP_fast_assign(
- strlcpy(__entry->name, bdi_dev_name(bdi),
+ strscpy(__entry->name, bdi_dev_name(bdi),
ARRAY_SIZE(__entry->name));
__entry->msg = msg;
__entry->step = step;
@@ -141,7 +141,7 @@ TRACE_EVENT(wbt_timer,
),

TP_fast_assign(
- strlcpy(__entry->name, bdi_dev_name(bdi),
+ strscpy(__entry->name, bdi_dev_name(bdi),
ARRAY_SIZE(__entry->name));
__entry->status = status;
__entry->step = step;
--
2.41.0.255.g8b1d071c50-goog


2023-07-12 23:55:52

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH 0/2] kyber, blk-wbt: Replace strlcpy with strscpy

On Mon, Jul 03, 2023 at 05:21:57PM +0000, Azeem Shaikh wrote:
> This patch series replaces strlcpy in the kyber and blk-wbt tracing subsystems wherever trivial
> replacement is possible, i.e return value from strlcpy is unused. The patches
> themselves are independent of each other and are applied to different subsystems. They are
> included as a series for ease of review.
>
> Note to reviewers: MAINTAINERS file does not specify clear ownership of
> these files so I have addressed these to the latest committer for these
> files: Jens Axboe <[email protected]>.
>
> Azeem Shaikh (2):
> kyber: Replace strlcpy with strscpy
> blk-wbt: Replace strlcpy with strscpy
>
> include/trace/events/kyber.h | 8 ++++----
> include/trace/events/wbt.h | 8 ++++----
> 2 files changed, 8 insertions(+), 8 deletions(-)

These looks correct to me. I can take them if no one else wants them.

Reviewed-by: Kees Cook <[email protected]>

--
Kees Cook

2023-07-13 02:17:44

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH 0/2] kyber, blk-wbt: Replace strlcpy with strscpy


On Mon, 03 Jul 2023 17:21:57 +0000, Azeem Shaikh wrote:
> This patch series replaces strlcpy in the kyber and blk-wbt tracing subsystems wherever trivial
> replacement is possible, i.e return value from strlcpy is unused. The patches
> themselves are independent of each other and are applied to different subsystems. They are
> included as a series for ease of review.
>
> Note to reviewers: MAINTAINERS file does not specify clear ownership of
> these files so I have addressed these to the latest committer for these
> files: Jens Axboe <[email protected]>.
>
> [...]

Applied, thanks!

[1/2] kyber: Replace strlcpy with strscpy
commit: 150b5f497df6bdb4730cae1eb98f13017b2eef6c
[2/2] blk-wbt: Replace strlcpy with strscpy
commit: 3abf6029341f2fd2ea6e48289662e8e4e3e96945

Best regards,
--
Jens Axboe