* [PATCH] drivers/xen/xenbus: Replace deprecated strcpy in xenbus_transaction_end
@ 2025-10-12 19:55 Thorsten Blum
2025-10-13 6:59 ` Jan Beulich
0 siblings, 1 reply; 6+ messages in thread
From: Thorsten Blum @ 2025-10-12 19:55 UTC (permalink / raw)
To: Juergen Gross, Stefano Stabellini, Oleksandr Tyshchenko,
Jason Andryuk, Dr. David Alan Gilbert
Cc: linux-hardening, Thorsten Blum, xen-devel, linux-kernel
strcpy() is deprecated; use strscpy() instead. Fix the function comment
and use bool instead of int while we're at it.
Link: https://github.com/KSPP/linux/issues/88
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
drivers/xen/xenbus/xenbus_xs.c | 9 +++------
include/xen/xenbus.h | 2 +-
2 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/xen/xenbus/xenbus_xs.c b/drivers/xen/xenbus/xenbus_xs.c
index 528682bf0c7f..970302b3dcc6 100644
--- a/drivers/xen/xenbus/xenbus_xs.c
+++ b/drivers/xen/xenbus/xenbus_xs.c
@@ -546,16 +546,13 @@ int xenbus_transaction_start(struct xenbus_transaction *t)
EXPORT_SYMBOL_GPL(xenbus_transaction_start);
/* End a transaction.
- * If abandon is true, transaction is discarded instead of committed.
+ * If abort is true, transaction is discarded instead of committed.
*/
-int xenbus_transaction_end(struct xenbus_transaction t, int abort)
+int xenbus_transaction_end(struct xenbus_transaction t, bool abort)
{
char abortstr[2];
- if (abort)
- strcpy(abortstr, "F");
- else
- strcpy(abortstr, "T");
+ strscpy(abortstr, abort ? "F" : "T");
return xs_error(xs_single(t, XS_TRANSACTION_END, abortstr, NULL));
}
diff --git a/include/xen/xenbus.h b/include/xen/xenbus.h
index 7dab04cf4a36..c94caf852aea 100644
--- a/include/xen/xenbus.h
+++ b/include/xen/xenbus.h
@@ -158,7 +158,7 @@ int xenbus_exists(struct xenbus_transaction t,
const char *dir, const char *node);
int xenbus_rm(struct xenbus_transaction t, const char *dir, const char *node);
int xenbus_transaction_start(struct xenbus_transaction *t);
-int xenbus_transaction_end(struct xenbus_transaction t, int abort);
+int xenbus_transaction_end(struct xenbus_transaction t, bool abort);
/* Single read and scanf: returns -errno or num scanned if > 0. */
__scanf(4, 5)
--
2.51.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drivers/xen/xenbus: Replace deprecated strcpy in xenbus_transaction_end
2025-10-12 19:55 [PATCH] drivers/xen/xenbus: Replace deprecated strcpy in xenbus_transaction_end Thorsten Blum
@ 2025-10-13 6:59 ` Jan Beulich
2025-10-13 7:36 ` Jürgen Groß
0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2025-10-13 6:59 UTC (permalink / raw)
To: Thorsten Blum
Cc: linux-hardening, xen-devel, linux-kernel, Juergen Gross,
Stefano Stabellini, Oleksandr Tyshchenko, Jason Andryuk,
Dr. David Alan Gilbert
On 12.10.2025 21:55, Thorsten Blum wrote:
> --- a/drivers/xen/xenbus/xenbus_xs.c
> +++ b/drivers/xen/xenbus/xenbus_xs.c
> @@ -546,16 +546,13 @@ int xenbus_transaction_start(struct xenbus_transaction *t)
> EXPORT_SYMBOL_GPL(xenbus_transaction_start);
>
> /* End a transaction.
> - * If abandon is true, transaction is discarded instead of committed.
> + * If abort is true, transaction is discarded instead of committed.
> */
> -int xenbus_transaction_end(struct xenbus_transaction t, int abort)
> +int xenbus_transaction_end(struct xenbus_transaction t, bool abort)
> {
> char abortstr[2];
>
> - if (abort)
> - strcpy(abortstr, "F");
> - else
> - strcpy(abortstr, "T");
While at least in principle a compiler might be able to transform this into
code not using any library function at all, ...
> + strscpy(abortstr, abort ? "F" : "T");
... the use of a n on-standard function (without equivalent compiler builtin)
doesn't permit this. IOW why not simply switch to e.g.
char abortstr[2] = { [0] = abort ? 'F' : 'T' };
Jan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drivers/xen/xenbus: Replace deprecated strcpy in xenbus_transaction_end
2025-10-13 6:59 ` Jan Beulich
@ 2025-10-13 7:36 ` Jürgen Groß
2025-10-13 8:22 ` Jan Beulich
0 siblings, 1 reply; 6+ messages in thread
From: Jürgen Groß @ 2025-10-13 7:36 UTC (permalink / raw)
To: Jan Beulich, Thorsten Blum
Cc: linux-hardening, xen-devel, linux-kernel, Stefano Stabellini,
Oleksandr Tyshchenko, Jason Andryuk, Dr. David Alan Gilbert
[-- Attachment #1.1.1: Type: text/plain, Size: 1955 bytes --]
On 13.10.25 08:59, Jan Beulich wrote:
> On 12.10.2025 21:55, Thorsten Blum wrote:
>> --- a/drivers/xen/xenbus/xenbus_xs.c
>> +++ b/drivers/xen/xenbus/xenbus_xs.c
>> @@ -546,16 +546,13 @@ int xenbus_transaction_start(struct xenbus_transaction *t)
>> EXPORT_SYMBOL_GPL(xenbus_transaction_start);
>>
>> /* End a transaction.
>> - * If abandon is true, transaction is discarded instead of committed.
>> + * If abort is true, transaction is discarded instead of committed.
>> */
>> -int xenbus_transaction_end(struct xenbus_transaction t, int abort)
>> +int xenbus_transaction_end(struct xenbus_transaction t, bool abort)
>> {
>> char abortstr[2];
>>
>> - if (abort)
>> - strcpy(abortstr, "F");
>> - else
>> - strcpy(abortstr, "T");
>
> While at least in principle a compiler might be able to transform this into
> code not using any library function at all, ...
>
>> + strscpy(abortstr, abort ? "F" : "T");
>
> ... the use of a n on-standard function (without equivalent compiler builtin)
> doesn't permit this. IOW why not simply switch to e.g.
>
> char abortstr[2] = { [0] = abort ? 'F' : 'T' };
I would even go further and drop abortstr[] completely:
diff --git a/drivers/xen/xenbus/xenbus_xs.c b/drivers/xen/xenbus/xenbus_xs.c
index 528682bf0c7f..c891af7165f5 100644
--- a/drivers/xen/xenbus/xenbus_xs.c
+++ b/drivers/xen/xenbus/xenbus_xs.c
@@ -550,14 +550,8 @@ EXPORT_SYMBOL_GPL(xenbus_transaction_start);
*/
int xenbus_transaction_end(struct xenbus_transaction t, int abort)
{
- char abortstr[2];
-
- if (abort)
- strcpy(abortstr, "F");
- else
- strcpy(abortstr, "T");
-
- return xs_error(xs_single(t, XS_TRANSACTION_END, abortstr, NULL));
+ return xs_error(xs_single(t, XS_TRANSACTION_END, abort ? "F" : "T",
+ NULL));
}
EXPORT_SYMBOL_GPL(xenbus_transaction_end);
Juergen
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drivers/xen/xenbus: Replace deprecated strcpy in xenbus_transaction_end
2025-10-13 7:36 ` Jürgen Groß
@ 2025-10-13 8:22 ` Jan Beulich
2025-10-13 9:24 ` Jürgen Groß
0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2025-10-13 8:22 UTC (permalink / raw)
To: Jürgen Groß
Cc: linux-hardening, xen-devel, linux-kernel, Stefano Stabellini,
Oleksandr Tyshchenko, Jason Andryuk, Dr. David Alan Gilbert,
Thorsten Blum
On 13.10.2025 09:36, Jürgen Groß wrote:
> On 13.10.25 08:59, Jan Beulich wrote:
>> On 12.10.2025 21:55, Thorsten Blum wrote:
>>> --- a/drivers/xen/xenbus/xenbus_xs.c
>>> +++ b/drivers/xen/xenbus/xenbus_xs.c
>>> @@ -546,16 +546,13 @@ int xenbus_transaction_start(struct xenbus_transaction *t)
>>> EXPORT_SYMBOL_GPL(xenbus_transaction_start);
>>>
>>> /* End a transaction.
>>> - * If abandon is true, transaction is discarded instead of committed.
>>> + * If abort is true, transaction is discarded instead of committed.
>>> */
>>> -int xenbus_transaction_end(struct xenbus_transaction t, int abort)
>>> +int xenbus_transaction_end(struct xenbus_transaction t, bool abort)
>>> {
>>> char abortstr[2];
>>>
>>> - if (abort)
>>> - strcpy(abortstr, "F");
>>> - else
>>> - strcpy(abortstr, "T");
>>
>> While at least in principle a compiler might be able to transform this into
>> code not using any library function at all, ...
>>
>>> + strscpy(abortstr, abort ? "F" : "T");
>>
>> ... the use of a n on-standard function (without equivalent compiler builtin)
>> doesn't permit this. IOW why not simply switch to e.g.
>>
>> char abortstr[2] = { [0] = abort ? 'F' : 'T' };
>
> I would even go further and drop abortstr[] completely:
>
> diff --git a/drivers/xen/xenbus/xenbus_xs.c b/drivers/xen/xenbus/xenbus_xs.c
> index 528682bf0c7f..c891af7165f5 100644
> --- a/drivers/xen/xenbus/xenbus_xs.c
> +++ b/drivers/xen/xenbus/xenbus_xs.c
> @@ -550,14 +550,8 @@ EXPORT_SYMBOL_GPL(xenbus_transaction_start);
> */
> int xenbus_transaction_end(struct xenbus_transaction t, int abort)
> {
> - char abortstr[2];
> -
> - if (abort)
> - strcpy(abortstr, "F");
> - else
> - strcpy(abortstr, "T");
> -
> - return xs_error(xs_single(t, XS_TRANSACTION_END, abortstr, NULL));
> + return xs_error(xs_single(t, XS_TRANSACTION_END, abort ? "F" : "T",
> + NULL));
> }
> EXPORT_SYMBOL_GPL(xenbus_transaction_end);
Hmm, which xs_single() indeed takes a const char *, it then casts away const-
ness before handing to xs_talkv().
Jan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drivers/xen/xenbus: Replace deprecated strcpy in xenbus_transaction_end
2025-10-13 8:22 ` Jan Beulich
@ 2025-10-13 9:24 ` Jürgen Groß
2025-10-13 11:23 ` Thorsten Blum
0 siblings, 1 reply; 6+ messages in thread
From: Jürgen Groß @ 2025-10-13 9:24 UTC (permalink / raw)
To: Jan Beulich
Cc: linux-hardening, xen-devel, linux-kernel, Stefano Stabellini,
Oleksandr Tyshchenko, Jason Andryuk, Dr. David Alan Gilbert,
Thorsten Blum
[-- Attachment #1.1.1: Type: text/plain, Size: 2460 bytes --]
On 13.10.25 10:22, Jan Beulich wrote:
> On 13.10.2025 09:36, Jürgen Groß wrote:
>> On 13.10.25 08:59, Jan Beulich wrote:
>>> On 12.10.2025 21:55, Thorsten Blum wrote:
>>>> --- a/drivers/xen/xenbus/xenbus_xs.c
>>>> +++ b/drivers/xen/xenbus/xenbus_xs.c
>>>> @@ -546,16 +546,13 @@ int xenbus_transaction_start(struct xenbus_transaction *t)
>>>> EXPORT_SYMBOL_GPL(xenbus_transaction_start);
>>>>
>>>> /* End a transaction.
>>>> - * If abandon is true, transaction is discarded instead of committed.
>>>> + * If abort is true, transaction is discarded instead of committed.
>>>> */
>>>> -int xenbus_transaction_end(struct xenbus_transaction t, int abort)
>>>> +int xenbus_transaction_end(struct xenbus_transaction t, bool abort)
>>>> {
>>>> char abortstr[2];
>>>>
>>>> - if (abort)
>>>> - strcpy(abortstr, "F");
>>>> - else
>>>> - strcpy(abortstr, "T");
>>>
>>> While at least in principle a compiler might be able to transform this into
>>> code not using any library function at all, ...
>>>
>>>> + strscpy(abortstr, abort ? "F" : "T");
>>>
>>> ... the use of a n on-standard function (without equivalent compiler builtin)
>>> doesn't permit this. IOW why not simply switch to e.g.
>>>
>>> char abortstr[2] = { [0] = abort ? 'F' : 'T' };
>>
>> I would even go further and drop abortstr[] completely:
>>
>> diff --git a/drivers/xen/xenbus/xenbus_xs.c b/drivers/xen/xenbus/xenbus_xs.c
>> index 528682bf0c7f..c891af7165f5 100644
>> --- a/drivers/xen/xenbus/xenbus_xs.c
>> +++ b/drivers/xen/xenbus/xenbus_xs.c
>> @@ -550,14 +550,8 @@ EXPORT_SYMBOL_GPL(xenbus_transaction_start);
>> */
>> int xenbus_transaction_end(struct xenbus_transaction t, int abort)
>> {
>> - char abortstr[2];
>> -
>> - if (abort)
>> - strcpy(abortstr, "F");
>> - else
>> - strcpy(abortstr, "T");
>> -
>> - return xs_error(xs_single(t, XS_TRANSACTION_END, abortstr, NULL));
>> + return xs_error(xs_single(t, XS_TRANSACTION_END, abort ? "F" : "T",
>> + NULL));
>> }
>> EXPORT_SYMBOL_GPL(xenbus_transaction_end);
>
> Hmm, which xs_single() indeed takes a const char *, it then casts away const-
> ness before handing to xs_talkv().
Yes, the cast is needed as xs_talkv() can handle reads and writes. No problem in
this case, as the string is only read by xs_talkv() (write type operation).
Juergen
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drivers/xen/xenbus: Replace deprecated strcpy in xenbus_transaction_end
2025-10-13 9:24 ` Jürgen Groß
@ 2025-10-13 11:23 ` Thorsten Blum
0 siblings, 0 replies; 6+ messages in thread
From: Thorsten Blum @ 2025-10-13 11:23 UTC (permalink / raw)
To: Jürgen Groß
Cc: Jan Beulich, linux-hardening, xen-devel, linux-kernel,
Stefano Stabellini, Oleksandr Tyshchenko, Jason Andryuk,
Dr. David Alan Gilbert
On 13. Oct 2025, at 11:24, Jürgen Groß wrote:
> On 13.10.25 10:22, Jan Beulich wrote:
>> On 13.10.2025 09:36, Jürgen Groß wrote:
>>> I would even go further and drop abortstr[] completely:
>>>
>>> diff --git a/drivers/xen/xenbus/xenbus_xs.c b/drivers/xen/xenbus/xenbus_xs.c
>>> index 528682bf0c7f..c891af7165f5 100644
>>> --- a/drivers/xen/xenbus/xenbus_xs.c
>>> +++ b/drivers/xen/xenbus/xenbus_xs.c
>>> @@ -550,14 +550,8 @@ EXPORT_SYMBOL_GPL(xenbus_transaction_start);
>>> */
>>> int xenbus_transaction_end(struct xenbus_transaction t, int abort)
>>> {
>>> - char abortstr[2];
>>> -
>>> - if (abort)
>>> - strcpy(abortstr, "F");
>>> - else
>>> - strcpy(abortstr, "T");
>>> -
>>> - return xs_error(xs_single(t, XS_TRANSACTION_END, abortstr, NULL));
>>> + return xs_error(xs_single(t, XS_TRANSACTION_END, abort ? "F" : "T",
>>> + NULL));
>>> }
>>> EXPORT_SYMBOL_GPL(xenbus_transaction_end);
>> Hmm, which xs_single() indeed takes a const char *, it then casts away const-
>> ness before handing to xs_talkv().
>
> Yes, the cast is needed as xs_talkv() can handle reads and writes. No problem in
> this case, as the string is only read by xs_talkv() (write type operation).
I'll submit a v2.
Thanks,
Thorsten
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-10-13 11:23 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-12 19:55 [PATCH] drivers/xen/xenbus: Replace deprecated strcpy in xenbus_transaction_end Thorsten Blum
2025-10-13 6:59 ` Jan Beulich
2025-10-13 7:36 ` Jürgen Groß
2025-10-13 8:22 ` Jan Beulich
2025-10-13 9:24 ` Jürgen Groß
2025-10-13 11:23 ` Thorsten Blum
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).