Messages in this thread | | | Date | Mon, 6 May 2019 09:56:58 -0700 | From | Jeremy Allison <> | Subject | Re: [PATCH] cifs: fix strcat buffer overflow in smb21_set_oplock_level() |
| |
On Mon, May 06, 2019 at 11:53:44AM -0500, Steve French via samba-technical wrote: > I think strcpy is clearer - but I don't think it can overflow since if > R, W or W were written to "message" then cinode->oplock would be > non-zero so we would never strcap "None"
Ahem. In Samba we have :
lib/util/safe_string.h:#define strcpy(dest,src) __ERROR__XX__NEVER_USE_STRCPY___;
Maybe you should do likewise :-).
> On Mon, May 6, 2019 at 10:26 AM Christoph Probst <kernel@probst.it> wrote: > > > > Change strcat to strcpy in the "None" case as it is never valid to append > > "None" to any other message. It may also overflow char message[5], in a > > race condition on cinode if cinode->oplock is unset by another thread > > after "RHW" or "RH" had been written to message. > > > > Signed-off-by: Christoph Probst <kernel@probst.it> > > --- > > fs/cifs/smb2ops.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c > > index c36ff0d..5fd5567 100644 > > --- a/fs/cifs/smb2ops.c > > +++ b/fs/cifs/smb2ops.c > > @@ -2936,7 +2936,7 @@ smb21_set_oplock_level(struct cifsInodeInfo *cinode, __u32 oplock, > > strcat(message, "W"); > > } > > if (!cinode->oplock) > > - strcat(message, "None"); > > + strcpy(message, "None"); > > cifs_dbg(FYI, "%s Lease granted on inode %p\n", message, > > &cinode->vfs_inode); > > } > > -- > > 2.1.4 > > > > > -- > Thanks, > > Steve >
| |