netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Bluetooth: replace deprecated strncpy with strscpy_pad
@ 2024-09-05 22:54 Justin Stitt
  2024-09-10 15:00 ` patchwork-bot+bluetooth
  0 siblings, 1 reply; 2+ messages in thread
From: Justin Stitt @ 2024-09-05 22:54 UTC (permalink / raw)
  To: Karsten Keil, Marcel Holtmann, Johan Hedberg,
	Luiz Augusto von Dentz
  Cc: netdev, linux-bluetooth, linux-kernel, Kees Cook, linux-hardening,
	Justin Stitt

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(-)

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;
 		}
 

---
base-commit: 521b1e7f4cf0b05a47995b103596978224b380a8
change-id: 20240905-strncpy-net-bluetooth-cmtp-capi-c-85b23d830279

Best regards,
--
Justin Stitt <justinstitt@google.com>


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] Bluetooth: replace deprecated strncpy with strscpy_pad
  2024-09-05 22:54 [PATCH] Bluetooth: replace deprecated strncpy with strscpy_pad Justin Stitt
@ 2024-09-10 15:00 ` patchwork-bot+bluetooth
  0 siblings, 0 replies; 2+ messages in thread
From: patchwork-bot+bluetooth @ 2024-09-10 15:00 UTC (permalink / raw)
  To: Justin Stitt
  Cc: isdn, marcel, johan.hedberg, luiz.dentz, netdev, linux-bluetooth,
	linux-kernel, kees, linux-hardening

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!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2024-09-10 15:00 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-05 22:54 [PATCH] Bluetooth: replace deprecated strncpy with strscpy_pad Justin Stitt
2024-09-10 15:00 ` patchwork-bot+bluetooth

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).