| Message ID | 20240905-strncpy-net-bluetooth-cmtp-capi-c-v1-1-c2d49caa2d36@google.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 278dcc36b992c205c4e6b1b35bd3c8f56716f820 |
| Headers | show |
| Series | Bluetooth: replace deprecated strncpy with strscpy_pad | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/CheckPatch | warning | WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?) #104: and as such we should prefer more robust and less ambiguous string interfaces. total: 0 errors, 1 warnings, 0 checks, 44 lines checked NOTE: For some of the reported defects, checkpatch may be able to mechanically convert to the typical style using --fix or --fix-inplace. /github/workspace/src/src/13793059.patch has style problems, please review. NOTE: Ignored message types: UNKNOWN_COMMIT_ID NOTE: If any of the errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS. |
| tedd_an/GitLint | fail | WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search 19: B1 Line exceeds max length (106>80): "Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [0]" |
| tedd_an/SubjectPrefix | success | Gitlint PASS |
| tedd_an/BuildKernel | success | BuildKernel PASS |
| tedd_an/CheckAllWarning | success | CheckAllWarning PASS |
| tedd_an/CheckSparse | success | CheckSparse PASS |
Hello: This patch was applied to bluetooth/bluetooth-next.git (master) by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>: On Thu, 05 Sep 2024 15:54:40 -0700 you wrote: > strncpy() is deprecated for use on NUL-terminated destination strings [0] > and as such we should prefer more robust and less ambiguous string interfaces. > > The CAPI (part II) [1] states that the manufacturer id should be a > "zero-terminated ASCII string" and should "always [be] zero-terminated." > > Much the same for the serial number: "The serial number, a seven-digit > number coded as a zero-terminated ASCII string". > > [...] Here is the summary with links: - Bluetooth: replace deprecated strncpy with strscpy_pad https://git.kernel.org/bluetooth/bluetooth-next/c/278dcc36b992 You are awesome, thank you!
diff --git a/net/bluetooth/cmtp/capi.c b/net/bluetooth/cmtp/capi.c index f3bedc3b613a..884703fda979 100644 --- a/net/bluetooth/cmtp/capi.c +++ b/net/bluetooth/cmtp/capi.c @@ -248,18 +248,10 @@ static void cmtp_recv_interopmsg(struct cmtp_session *session, struct sk_buff *s break; case CAPI_FUNCTION_GET_MANUFACTURER: - if (skb->len < CAPI_MSG_BASELEN + 15) - break; - - if (!info && ctrl) { - int len = min_t(uint, CAPI_MANUFACTURER_LEN, - skb->data[CAPI_MSG_BASELEN + 14]); - - memset(ctrl->manu, 0, CAPI_MANUFACTURER_LEN); - strncpy(ctrl->manu, - skb->data + CAPI_MSG_BASELEN + 15, len); - } - + if (!info && ctrl && skb->len > CAPI_MSG_BASELEN + 14) + strscpy_pad(ctrl->manu, + skb->data + CAPI_MSG_BASELEN + 15, + skb->data[CAPI_MSG_BASELEN + 14]); break; case CAPI_FUNCTION_GET_VERSION: @@ -276,18 +268,10 @@ static void cmtp_recv_interopmsg(struct cmtp_session *session, struct sk_buff *s break; case CAPI_FUNCTION_GET_SERIAL_NUMBER: - if (skb->len < CAPI_MSG_BASELEN + 17) - break; - - if (!info && ctrl) { - int len = min_t(uint, CAPI_SERIAL_LEN, - skb->data[CAPI_MSG_BASELEN + 16]); - - memset(ctrl->serial, 0, CAPI_SERIAL_LEN); - strncpy(ctrl->serial, - skb->data + CAPI_MSG_BASELEN + 17, len); - } - + if (!info && ctrl && skb->len > CAPI_MSG_BASELEN + 16) + strscpy_pad(ctrl->serial, + skb->data + CAPI_MSG_BASELEN + 17, + skb->data[CAPI_MSG_BASELEN + 16]); break; }
strncpy() is deprecated for use on NUL-terminated destination strings [0] and as such we should prefer more robust and less ambiguous string interfaces. The CAPI (part II) [1] states that the manufacturer id should be a "zero-terminated ASCII string" and should "always [be] zero-terminated." Much the same for the serial number: "The serial number, a seven-digit number coded as a zero-terminated ASCII string". Along with this, its clear the original author intended for these buffers to be NUL-padded as well. To meet the specification as well as properly NUL-pad, use strscpy_pad(). In doing this, an opportunity to simplify this code is also present. Remove the min_t() and combine the length check into the main if. Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [0] Link: https://capi.org/downloads.html [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-hardening@vger.kernel.org Signed-off-by: Justin Stitt <justinstitt@google.com> --- Note: build-tested only. For future travelers: skb->data + CAPI_MSG_BASELN + 14 is a byte that specifies the length of the message to follow, and of course "... + 15" is the offset of the message itself. Due to this, we cannot use the more appropriate memtostr_pad() because we don't know all the sizes at compile time. --- net/bluetooth/cmtp/capi.c | 32 ++++++++------------------------ 1 file changed, 8 insertions(+), 24 deletions(-) --- base-commit: 521b1e7f4cf0b05a47995b103596978224b380a8 change-id: 20240905-strncpy-net-bluetooth-cmtp-capi-c-85b23d830279 Best regards, -- Justin Stitt <justinstitt@google.com>