Messages in this thread | | | From | Doug Anderson <> | Date | Mon, 27 Jun 2022 16:06:49 -0700 | Subject | Re: [PATCH v2 1/2] soc: qcom: cmd-db: replace strncpy() with strscpy_pad() |
| |
Hi,
On Thu, May 19, 2022 at 12:33 AM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > The use of strncpy() is considered deprecated for NUL-terminated > strings[1]. Replace strncpy() with strscpy_pad(), to keep existing > pad-behavior of strncpy. This fixes W=1 warning: > > drivers/soc/qcom/cmd-db.c: In function ‘cmd_db_get_header.part.0’: > drivers/soc/qcom/cmd-db.c:151:9: warning: ‘strncpy’ specified bound 8 equals destination size [-Wstringop-truncation] > 151 | strncpy(query, id, sizeof(query)); > > [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings > > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Reviewed-by: Stephen Boyd <sboyd@kernel.org> > > --- > > Changes since v1: > 1. Split series per subsystem. > 2. Add tag. > --- > drivers/soc/qcom/cmd-db.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/soc/qcom/cmd-db.c b/drivers/soc/qcom/cmd-db.c > index dd872017f345..c5137c25d819 100644 > --- a/drivers/soc/qcom/cmd-db.c > +++ b/drivers/soc/qcom/cmd-db.c > @@ -148,7 +148,7 @@ static int cmd_db_get_header(const char *id, const struct entry_header **eh, > return ret; > > /* Pad out query string to same length as in DB */ > - strncpy(query, id, sizeof(query)); > + strscpy_pad(query, id, sizeof(query));
Sorry to report that this breaks booting on sc7280-herobrine-herobrine-r1. I believe that the function was actually _relying_ on the "unsafe" behavior of strncpy(). Specifically I think: * The source string (id) was a '\0' terminated string. * The destination (query) was a fixed 8-byte string and doesn't need to be '\0' terminated.
So before your change we could actually support 8-byte strings. Now we can't because you'll only copy 7 bytes to the destination to leave room for the '\0' termination...
Looking at printouts, I see, for instance at least one ID that looks like "lnbclka2".
Given the requirements of this function(), the old strncpy() is actually _exactly_ what we want. Is there any way to disable the warning? If not, I guess we could make "query" be 9 bytes bit, or "sizeof(ent->id) + 1" bytes big... Happy to post a patch, but it's basically a bikeshed for how you want it fixed (there are dozens of ways) and I'd rather you just tell me instead of me guessing.
-Doug
| |