| Message ID | 20240318-strncpy-drivers-thermal-intel-int340x_thermal-acpi_thermal_rel-c-v1-1-08839fbf737a@google.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 03fa9a3ad1d61992a2105aeb1062b349f1a85012 |
| Headers | show |
| Series | thermal: intel: int340x_thermal: replace deprecated strncpy with strscpy | expand |
On Mon, Mar 18, 2024 at 11:36 PM Justin Stitt <justinstitt@google.com> 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. > > psvt->limit.string can only be 8 bytes so let's use the appropriate size > macro ACPI_LIMIT_STR_MAX_LEN. > > Neither psvt->limit.string or psvt_user[i].limit.string requires the > NUL-padding behavior that strncpy() provides as they have both been > filled with NUL-bytes prior to the string operation. > | memset(&psvt->limit, 0, sizeof(u64)); > and > | psvt_user = kzalloc(psvt_len, GFP_KERNEL); > > Let's use `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-hardening@vger.kernel.org > Signed-off-by: Justin Stitt <justinstitt@google.com> Srinivas, any objections? > --- > Note: build-tested only. > > Found with: $ rg "strncpy\(" > --- > drivers/thermal/intel/int340x_thermal/acpi_thermal_rel.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/thermal/intel/int340x_thermal/acpi_thermal_rel.c b/drivers/thermal/intel/int340x_thermal/acpi_thermal_rel.c > index dc519a665c18..4b4a4d63e61f 100644 > --- a/drivers/thermal/intel/int340x_thermal/acpi_thermal_rel.c > +++ b/drivers/thermal/intel/int340x_thermal/acpi_thermal_rel.c > @@ -309,7 +309,7 @@ static int acpi_parse_psvt(acpi_handle handle, int *psvt_count, struct psvt **ps > > if (knob->type == ACPI_TYPE_STRING) { > memset(&psvt->limit, 0, sizeof(u64)); > - strncpy(psvt->limit.string, psvt_ptr->limit.str_ptr, knob->string.length); > + strscpy(psvt->limit.string, psvt_ptr->limit.str_ptr, ACPI_LIMIT_STR_MAX_LEN); > } else { > psvt->limit.integer = psvt_ptr->limit.integer; > } > @@ -468,7 +468,7 @@ static int fill_psvt(char __user *ubuf) > psvt_user[i].unlimit_coeff = psvts[i].unlimit_coeff; > psvt_user[i].control_knob_type = psvts[i].control_knob_type; > if (psvt_user[i].control_knob_type == ACPI_TYPE_STRING) > - strncpy(psvt_user[i].limit.string, psvts[i].limit.string, > + strscpy(psvt_user[i].limit.string, psvts[i].limit.string, > ACPI_LIMIT_STR_MAX_LEN); > else > psvt_user[i].limit.integer = psvts[i].limit.integer; > > --- > base-commit: bf3a69c6861ff4dc7892d895c87074af7bc1c400 > change-id: 20240318-strncpy-drivers-thermal-intel-int340x_thermal-acpi_thermal_rel-c-17070c1e42f3 > > Best regards, > -- > Justin Stitt <justinstitt@google.com> >
On Tue, 2024-03-19 at 12:39 +0100, Rafael J. Wysocki wrote: > On Mon, Mar 18, 2024 at 11:36 PM Justin Stitt > <justinstitt@google.com> 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. > > > > psvt->limit.string can only be 8 bytes so let's use the appropriate > > size > > macro ACPI_LIMIT_STR_MAX_LEN. > > > > Neither psvt->limit.string or psvt_user[i].limit.string requires > > the > > NUL-padding behavior that strncpy() provides as they have both been > > filled with NUL-bytes prior to the string operation. > > > memset(&psvt->limit, 0, sizeof(u64)); > > and > > > psvt_user = kzalloc(psvt_len, GFP_KERNEL); > > > > Let's use `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-hardening@vger.kernel.org > > Signed-off-by: Justin Stitt <justinstitt@google.com> > > Srinivas, any objections? No Reviewed-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> > > > --- > > Note: build-tested only. > > > > Found with: $ rg "strncpy\(" > > --- > > drivers/thermal/intel/int340x_thermal/acpi_thermal_rel.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git > > a/drivers/thermal/intel/int340x_thermal/acpi_thermal_rel.c > > b/drivers/thermal/intel/int340x_thermal/acpi_thermal_rel.c > > index dc519a665c18..4b4a4d63e61f 100644 > > --- a/drivers/thermal/intel/int340x_thermal/acpi_thermal_rel.c > > +++ b/drivers/thermal/intel/int340x_thermal/acpi_thermal_rel.c > > @@ -309,7 +309,7 @@ static int acpi_parse_psvt(acpi_handle handle, > > int *psvt_count, struct psvt **ps > > > > if (knob->type == ACPI_TYPE_STRING) { > > memset(&psvt->limit, 0, sizeof(u64)); > > - strncpy(psvt->limit.string, psvt_ptr- > > >limit.str_ptr, knob->string.length); > > + strscpy(psvt->limit.string, psvt_ptr- > > >limit.str_ptr, ACPI_LIMIT_STR_MAX_LEN); > > } else { > > psvt->limit.integer = psvt_ptr- > > >limit.integer; > > } > > @@ -468,7 +468,7 @@ static int fill_psvt(char __user *ubuf) > > psvt_user[i].unlimit_coeff = > > psvts[i].unlimit_coeff; > > psvt_user[i].control_knob_type = > > psvts[i].control_knob_type; > > if (psvt_user[i].control_knob_type == > > ACPI_TYPE_STRING) > > - strncpy(psvt_user[i].limit.string, > > psvts[i].limit.string, > > + strscpy(psvt_user[i].limit.string, > > psvts[i].limit.string, > > ACPI_LIMIT_STR_MAX_LEN); > > else > > psvt_user[i].limit.integer = > > psvts[i].limit.integer; > > > > --- > > base-commit: bf3a69c6861ff4dc7892d895c87074af7bc1c400 > > change-id: 20240318-strncpy-drivers-thermal-intel-int340x_thermal- > > acpi_thermal_rel-c-17070c1e42f3 > > > > Best regards, > > -- > > Justin Stitt <justinstitt@google.com> > >
On Tue, Mar 19, 2024 at 5:20 PM srinivas pandruvada <srinivas.pandruvada@linux.intel.com> wrote: > > On Tue, 2024-03-19 at 12:39 +0100, Rafael J. Wysocki wrote: > > On Mon, Mar 18, 2024 at 11:36 PM Justin Stitt > > <justinstitt@google.com> 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. > > > > > > psvt->limit.string can only be 8 bytes so let's use the appropriate > > > size > > > macro ACPI_LIMIT_STR_MAX_LEN. > > > > > > Neither psvt->limit.string or psvt_user[i].limit.string requires > > > the > > > NUL-padding behavior that strncpy() provides as they have both been > > > filled with NUL-bytes prior to the string operation. > > > > memset(&psvt->limit, 0, sizeof(u64)); > > > and > > > > psvt_user = kzalloc(psvt_len, GFP_KERNEL); > > > > > > Let's use `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-hardening@vger.kernel.org > > > Signed-off-by: Justin Stitt <justinstitt@google.com> > > > > Srinivas, any objections? > No > > Reviewed-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> Applied as 6.10 material, thanks!
diff --git a/drivers/thermal/intel/int340x_thermal/acpi_thermal_rel.c b/drivers/thermal/intel/int340x_thermal/acpi_thermal_rel.c index dc519a665c18..4b4a4d63e61f 100644 --- a/drivers/thermal/intel/int340x_thermal/acpi_thermal_rel.c +++ b/drivers/thermal/intel/int340x_thermal/acpi_thermal_rel.c @@ -309,7 +309,7 @@ static int acpi_parse_psvt(acpi_handle handle, int *psvt_count, struct psvt **ps if (knob->type == ACPI_TYPE_STRING) { memset(&psvt->limit, 0, sizeof(u64)); - strncpy(psvt->limit.string, psvt_ptr->limit.str_ptr, knob->string.length); + strscpy(psvt->limit.string, psvt_ptr->limit.str_ptr, ACPI_LIMIT_STR_MAX_LEN); } else { psvt->limit.integer = psvt_ptr->limit.integer; } @@ -468,7 +468,7 @@ static int fill_psvt(char __user *ubuf) psvt_user[i].unlimit_coeff = psvts[i].unlimit_coeff; psvt_user[i].control_knob_type = psvts[i].control_knob_type; if (psvt_user[i].control_knob_type == ACPI_TYPE_STRING) - strncpy(psvt_user[i].limit.string, psvts[i].limit.string, + strscpy(psvt_user[i].limit.string, psvts[i].limit.string, ACPI_LIMIT_STR_MAX_LEN); else psvt_user[i].limit.integer = psvts[i].limit.integer;
strncpy() is deprecated for use on NUL-terminated destination strings [1] and as such we should prefer more robust and less ambiguous string interfaces. psvt->limit.string can only be 8 bytes so let's use the appropriate size macro ACPI_LIMIT_STR_MAX_LEN. Neither psvt->limit.string or psvt_user[i].limit.string requires the NUL-padding behavior that strncpy() provides as they have both been filled with NUL-bytes prior to the string operation. | memset(&psvt->limit, 0, sizeof(u64)); and | psvt_user = kzalloc(psvt_len, GFP_KERNEL); Let's use `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-hardening@vger.kernel.org Signed-off-by: Justin Stitt <justinstitt@google.com> --- Note: build-tested only. Found with: $ rg "strncpy\(" --- drivers/thermal/intel/int340x_thermal/acpi_thermal_rel.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) --- base-commit: bf3a69c6861ff4dc7892d895c87074af7bc1c400 change-id: 20240318-strncpy-drivers-thermal-intel-int340x_thermal-acpi_thermal_rel-c-17070c1e42f3 Best regards, -- Justin Stitt <justinstitt@google.com>