On Wed, Mar 26, 2025 at 02:40:22PM +0000, Roxana Nicolescu wrote: > On Wednesday, March 26th, 2025 at 1:19 PM, Kent Overstreet > <[email protected]> wrote: > > > On Wed, Mar 26, 2025 at 06:02:31PM +0800, Alan Huang wrote: > > > > > On Mar 26, 2025, at 17:44, Roxana Nicolescu > > > [email protected] wrote: > > > > > > > strncpy() is deprecated for NUL-terminated destination buffers. Use > > > > strscpy() instead. > > > > > > > > Link: https://github.com/KSPP/linux/issues/90 > > > > Signed-off-by: Roxana Nicolescu [email protected] > > > > --- > > > > fs/bcachefs/btree_trans_commit.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/fs/bcachefs/btree_trans_commit.c > > > > b/fs/bcachefs/btree_trans_commit.c > > > > index c4f524b2ca9a..7ab25b425d11 100644 > > > > --- a/fs/bcachefs/btree_trans_commit.c > > > > +++ b/fs/bcachefs/btree_trans_commit.c > > > > @@ -364,7 +364,7 @@ static noinline void > > > > journal_transaction_name(struct btree_trans *trans) > > > > struct jset_entry_log *l = > > > > container_of(entry, struct jset_entry_log, entry); > > > > > > > > - strncpy(l->d, trans->fn, JSET_ENTRY_LOG_U64s * sizeof(u64)); > > > > + strscpy(l->d, trans->fn, JSET_ENTRY_LOG_U64s * sizeof(u64)); > > > > > > The last time I asked Kent about this line, he didn’t want this. > > > > > > Yes, the destination buffer isn't required to be nul terminated. > > > > But it seems we should add a comment explaining that :) > > I should have checked the mailing list before, but I will keep this in mind > for my next contributions. > I wonder if we should use memcpy in this case. This is also recommended by > the security team here https://github.com/KSPP/linux/issues/90 > This will also prevent other people from trying to send a similar patch in > the future.
Or better, a new helper: when we're copying to a fixed size buffer we really want to zero out the rest of the buffer - we don't just want a single terminating nul. This has come up in other places in bcachefs, see __bch2_fs_log_msg() in btree_update.c, and I would imagine the code for updating superblock labels as well.
