* [PATCH] fs: smb: client: Add missing check for kstrdup()
@ 2024-07-01 6:48 Haoxiang Li
2024-07-01 13:01 ` Markus Elfring
0 siblings, 1 reply; 6+ messages in thread
From: Haoxiang Li @ 2024-07-01 6:48 UTC (permalink / raw)
To: sfrench, pc, ronniesahlberg, sprasad, tom, bharathsm
Cc: linux-cifs, samba-technical, linux-kernel, Haoxiang Li
Add check for kstrdup() in smb3_reconfigure in order to guarantee
the success of allocation.
Fixes: c1eb537bf456 ("cifs: allow changing password during remount")
Signed-off-by: Haoxiang Li <make24@iscas.ac.cn>
---
fs/smb/client/fs_context.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c
index 3bbac925d076..8253b615b8ce 100644
--- a/fs/smb/client/fs_context.c
+++ b/fs/smb/client/fs_context.c
@@ -920,6 +920,8 @@ static int smb3_reconfigure(struct fs_context *fc)
ses->password = kstrdup(ctx->password, GFP_KERNEL);
kfree_sensitive(ses->password2);
ses->password2 = kstrdup(ctx->password2, GFP_KERNEL);
+ if (!ses->password || !ses->password2)
+ return ERR_PTR(rc);
}
STEAL_STRING(cifs_sb, ctx, domainname);
STEAL_STRING(cifs_sb, ctx, nodename);
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] fs: smb: client: Add missing check for kstrdup() 2024-07-01 6:48 [PATCH] fs: smb: client: Add missing check for kstrdup() Haoxiang Li @ 2024-07-01 13:01 ` Markus Elfring 2024-10-22 17:15 ` [PATCH v2] fs_context.c: smb3_reconfigure: Handle kstrdup failures for passwords Henrique Carvalho 0 siblings, 1 reply; 6+ messages in thread From: Markus Elfring @ 2024-07-01 13:01 UTC (permalink / raw) To: Haoxiang Li, samba-technical, linux-cifs, kernel-janitors, Bharath SM, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N, Steve French, Tom Talpey Cc: LKML > Add check for kstrdup() in smb3_reconfigure in order to guarantee checks? calls? () > the success of allocation. I suggest to take further patch/code review concerns better into account. … > Signed-off-by: Haoxiang Li <make24@iscas.ac.cn> Will requirements be reconsidered once more for the Developer's Certificate of Origin? https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.10-rc6#n398 How do you think about to use a summary phrase like “Complete error handling in smb3_reconfigure()”? … > +++ b/fs/smb/client/fs_context.c > @@ -920,6 +920,8 @@ static int smb3_reconfigure(struct fs_context *fc) > ses->password = kstrdup(ctx->password, GFP_KERNEL); > kfree_sensitive(ses->password2); > ses->password2 = kstrdup(ctx->password2, GFP_KERNEL); > + if (!ses->password || !ses->password2) > + return ERR_PTR(rc); > } … How do you think about to avoid also a memory leak here? Regards, Markus ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] fs_context.c: smb3_reconfigure: Handle kstrdup failures for passwords 2024-07-01 13:01 ` Markus Elfring @ 2024-10-22 17:15 ` Henrique Carvalho 2024-10-22 18:21 ` [PATCH v3] smb: client: " Henrique Carvalho 0 siblings, 1 reply; 6+ messages in thread From: Henrique Carvalho @ 2024-10-22 17:15 UTC (permalink / raw) To: markus.elfring Cc: bharathsm, kernel-janitors, linux-cifs, linux-kernel, make24, pc, ronniesahlberg, samba-technical, sfrench, sprasad, tom, ematsumiya, Henrique Carvalho In smb3_reconfigure(), after duplicating ctx->password and ctx->password2 with kstrdup(), we need to check for allocation failures. If ses->password allocation fails, return -ENOMEM. If ses->password2 allocation fails, free ses->password, set it to NULL, and return -ENOMEM. Fixes: c1eb537bf456 ("cifs: allow changing password during remount") Signed-off-by: Haoxiang Li <make24@iscas.ac.cn> Signed-off-by: Henrique Carvalho <henrique.carvalho@suse.com> --- V1 -> V2: Decoupled checks for ses->password and ses->password2. Ensured ses->password is freed and set to NULL if ses->password2 allocation fails. Corrected return value. Improved commit message. fs/smb/client/fs_context.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c index 28c4e576d460a..5c5a52019efad 100644 --- a/fs/smb/client/fs_context.c +++ b/fs/smb/client/fs_context.c @@ -920,8 +920,15 @@ static int smb3_reconfigure(struct fs_context *fc) else { kfree_sensitive(ses->password); ses->password = kstrdup(ctx->password, GFP_KERNEL); + if (!ses->password) + return -ENOMEM; kfree_sensitive(ses->password2); ses->password2 = kstrdup(ctx->password2, GFP_KERNEL); + if (!ses->password2) { + kfree_sensitive(ses->password); + ses->password = NULL; + return -ENOMEM; + } } STEAL_STRING(cifs_sb, ctx, domainname); STEAL_STRING(cifs_sb, ctx, nodename); -- 2.46.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3] smb: client: Handle kstrdup failures for passwords 2024-10-22 17:15 ` [PATCH v2] fs_context.c: smb3_reconfigure: Handle kstrdup failures for passwords Henrique Carvalho @ 2024-10-22 18:21 ` Henrique Carvalho 2024-10-22 22:21 ` Steve French 2024-11-04 11:06 ` Markus Elfring 0 siblings, 2 replies; 6+ messages in thread From: Henrique Carvalho @ 2024-10-22 18:21 UTC (permalink / raw) To: sfrench Cc: bharathsm, ematsumiya, kernel-janitors, linux-cifs, make24, markus.elfring, pc, ronniesahlberg, sprasad, tom, Henrique Carvalho In smb3_reconfigure(), after duplicating ctx->password and ctx->password2 with kstrdup(), we need to check for allocation failures. If ses->password allocation fails, return -ENOMEM. If ses->password2 allocation fails, free ses->password, set it to NULL, and return -ENOMEM. Fixes: c1eb537bf456 ("cifs: allow changing password during remount") Signed-off-by: Haoxiang Li <make24@iscas.ac.cn> Signed-off-by: Henrique Carvalho <henrique.carvalho@suse.com> --- V2 -> V3: Adjust commit subject. V1 -> V2: Decoupled checks for ses->password and ses->password2. Ensured ses->password is freed and set to NULL if ses->password2 allocation fails. Corrected return value. Improved commit message. fs/smb/client/fs_context.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c index 28c4e576d460a..5c5a52019efad 100644 --- a/fs/smb/client/fs_context.c +++ b/fs/smb/client/fs_context.c @@ -920,8 +920,15 @@ static int smb3_reconfigure(struct fs_context *fc) else { kfree_sensitive(ses->password); ses->password = kstrdup(ctx->password, GFP_KERNEL); + if (!ses->password) + return -ENOMEM; kfree_sensitive(ses->password2); ses->password2 = kstrdup(ctx->password2, GFP_KERNEL); + if (!ses->password2) { + kfree_sensitive(ses->password); + ses->password = NULL; + return -ENOMEM; + } } STEAL_STRING(cifs_sb, ctx, domainname); STEAL_STRING(cifs_sb, ctx, nodename); -- 2.46.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3] smb: client: Handle kstrdup failures for passwords 2024-10-22 18:21 ` [PATCH v3] smb: client: " Henrique Carvalho @ 2024-10-22 22:21 ` Steve French 2024-11-04 11:06 ` Markus Elfring 1 sibling, 0 replies; 6+ messages in thread From: Steve French @ 2024-10-22 22:21 UTC (permalink / raw) To: Henrique Carvalho Cc: sfrench, bharathsm, ematsumiya, kernel-janitors, linux-cifs, make24, markus.elfring, pc, ronniesahlberg, sprasad, tom tentatively merged into cifs-2.6.git for-next pending review and more testing On Tue, Oct 22, 2024 at 1:22 PM Henrique Carvalho <henrique.carvalho@suse.com> wrote: > > In smb3_reconfigure(), after duplicating ctx->password and > ctx->password2 with kstrdup(), we need to check for allocation > failures. > > If ses->password allocation fails, return -ENOMEM. > If ses->password2 allocation fails, free ses->password, set it > to NULL, and return -ENOMEM. > > Fixes: c1eb537bf456 ("cifs: allow changing password during remount") > Signed-off-by: Haoxiang Li <make24@iscas.ac.cn> > Signed-off-by: Henrique Carvalho <henrique.carvalho@suse.com> > --- > V2 -> V3: Adjust commit subject. > V1 -> V2: Decoupled checks for ses->password and ses->password2. Ensured > ses->password is freed and set to NULL if ses->password2 allocation > fails. Corrected return value. Improved commit message. > > fs/smb/client/fs_context.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c > index 28c4e576d460a..5c5a52019efad 100644 > --- a/fs/smb/client/fs_context.c > +++ b/fs/smb/client/fs_context.c > @@ -920,8 +920,15 @@ static int smb3_reconfigure(struct fs_context *fc) > else { > kfree_sensitive(ses->password); > ses->password = kstrdup(ctx->password, GFP_KERNEL); > + if (!ses->password) > + return -ENOMEM; > kfree_sensitive(ses->password2); > ses->password2 = kstrdup(ctx->password2, GFP_KERNEL); > + if (!ses->password2) { > + kfree_sensitive(ses->password); > + ses->password = NULL; > + return -ENOMEM; > + } > } > STEAL_STRING(cifs_sb, ctx, domainname); > STEAL_STRING(cifs_sb, ctx, nodename); > -- > 2.46.0 > > -- Thanks, Steve ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] smb: client: Handle kstrdup failures for passwords 2024-10-22 18:21 ` [PATCH v3] smb: client: " Henrique Carvalho 2024-10-22 22:21 ` Steve French @ 2024-11-04 11:06 ` Markus Elfring 1 sibling, 0 replies; 6+ messages in thread From: Markus Elfring @ 2024-11-04 11:06 UTC (permalink / raw) To: Henrique Carvalho, make24, linux-cifs, kernel-janitors, Steve French Cc: LKML, Bharath SM, Enzo Matsumiya, Paulo Alcantara, Ronnie Sahlberg, Shyam Prasad N, Steve French, Tom Talpey … > Signed-off-by: Haoxiang Li <make24@iscas.ac.cn> How relevant can such information still be according to requirements for the Developer's Certificate of Origin after different names were presented for such an email address? https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.12-rc6#n396 https://lore.kernel.org/linux-kernel/?q=make24%40iscas.ac.cn Regards, Markus ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-11-04 11:06 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-07-01 6:48 [PATCH] fs: smb: client: Add missing check for kstrdup() Haoxiang Li 2024-07-01 13:01 ` Markus Elfring 2024-10-22 17:15 ` [PATCH v2] fs_context.c: smb3_reconfigure: Handle kstrdup failures for passwords Henrique Carvalho 2024-10-22 18:21 ` [PATCH v3] smb: client: " Henrique Carvalho 2024-10-22 22:21 ` Steve French 2024-11-04 11:06 ` Markus Elfring
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox