On Wed, Aug 28, 2019 at 11:00 AM Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Tue, Aug 27, 2019 at 3:34 PM Steve French <smfrench@xxxxxxxxx> wrote:
> >
> > - } else { /* BB improve check for buffer overruns BB */
> > - name_len = strnlen(name, PATH_MAX);
> > - name_len++; /* trailing null */
> > - strncpy(pSMB->fileName, name, name_len);
> > + } else {
> > + name_len = copy_path_name(pSMB->fileName, name);
>
> Hmm. If you kept the PATH_MAX value as an argument, you could then ...
>
> > - strncpy(bcc_ptr, ses->user_name, CIFS_MAX_USERNAME_LEN);
> > - bcc_ptr += strnlen(ses->user_name, CIFS_MAX_USERNAME_LEN);
> > + len = strscpy(bcc_ptr, ses->user_name, CIFS_MAX_USERNAME_LEN);
> > + if (WARN_ON_ONCE(len < 0))
> > + len = CIFS_MAX_USERNAME_LEN - 1;
> > + bcc_ptr += len;
>
> ... have used that function here too, instead of open-coding it just
> because the max length is now CIFS_MAX_USERNAME_LEN.
>
> Although I guess that case is slightly different because it only adds
> "len", not including the final NUL in the count.
>
> So who knows. The patch looks like a clear improvement, although I
> think the smb1 code could have used a helper that did the UTF16 cases
> too, because now all _that_ code is still duplicated and I'm not
> convinced that gets the final NUL any more right..
Good points. I can do these improvements as a follow-on for 5.4
ronnie sahlberg
>
> Linus
