Re: svn commit: r368375 - head/sys/kern
Thanks for the report. Fixed in r368395.
On 12/6/20, Jessica Clarke wrote:
> Hi Mateusz,
> This looks like a behavioural change to me. Was that intended and a bug
> fix (in which case, it should have been documented in the commit
> message) or not? See below for the exact change.
>
> On 6 Dec 2020, at 04:59, Mateusz Guzik wrote:
>> +static int
>> +namei_getpath(struct nameidata *ndp)
>> +{
>> +struct componentname *cnp;
>> +int error;
>> +
>> +cnp = >ni_cnd;
>> +
>> +/*
>> + * Get a buffer for the name to be translated, and copy the
>> + * name into the buffer.
>> + */
>> +cnp->cn_pnbuf = uma_zalloc(namei_zone, M_WAITOK);
>> +if (ndp->ni_segflg == UIO_SYSSPACE) {
>> +error = copystr(ndp->ni_dirp, cnp->cn_pnbuf, MAXPATHLEN,
>> +>ni_pathlen);
>> +} else {
>> +error = copyinstr(ndp->ni_dirp, cnp->cn_pnbuf, MAXPATHLEN,
>> +>ni_pathlen);
>> +}
>> +
>> +if (__predict_false(error != 0)) {
>> +return (error);
>
> This does not call namei_cleanup_cnp.
>
>> @@ -531,31 +568,11 @@ namei(struct nameidata *ndp)
>> ndp->ni_lcf = 0;
>> ndp->ni_vp = NULL;
>>
>> -/*
>> - * Get a buffer for the name to be translated, and copy the
>> - * name into the buffer.
>> - */
>> -cnp->cn_pnbuf = uma_zalloc(namei_zone, M_WAITOK);
>> -if (ndp->ni_segflg == UIO_SYSSPACE)
>> -error = copystr(ndp->ni_dirp, cnp->cn_pnbuf, MAXPATHLEN,
>> ->ni_pathlen);
>> -else
>> -error = copyinstr(ndp->ni_dirp, cnp->cn_pnbuf, MAXPATHLEN,
>> ->ni_pathlen);
>> -
>> +error = namei_getpath(ndp);
>> if (__predict_false(error != 0)) {
>> -namei_cleanup_cnp(cnp);
>
> But it used to be called in that case here.
>
>> return (error);
>> }
>
> Jess
>
>
--
Mateusz Guzik
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r368375 - head/sys/kern
Hi Mateusz,
This looks like a behavioural change to me. Was that intended and a bug
fix (in which case, it should have been documented in the commit
message) or not? See below for the exact change.
On 6 Dec 2020, at 04:59, Mateusz Guzik wrote:
> +static int
> +namei_getpath(struct nameidata *ndp)
> +{
> + struct componentname *cnp;
> + int error;
> +
> + cnp = >ni_cnd;
> +
> + /*
> + * Get a buffer for the name to be translated, and copy the
> + * name into the buffer.
> + */
> + cnp->cn_pnbuf = uma_zalloc(namei_zone, M_WAITOK);
> + if (ndp->ni_segflg == UIO_SYSSPACE) {
> + error = copystr(ndp->ni_dirp, cnp->cn_pnbuf, MAXPATHLEN,
> + >ni_pathlen);
> + } else {
> + error = copyinstr(ndp->ni_dirp, cnp->cn_pnbuf, MAXPATHLEN,
> + >ni_pathlen);
> + }
> +
> + if (__predict_false(error != 0)) {
> + return (error);
This does not call namei_cleanup_cnp.
> @@ -531,31 +568,11 @@ namei(struct nameidata *ndp)
> ndp->ni_lcf = 0;
> ndp->ni_vp = NULL;
>
> - /*
> - * Get a buffer for the name to be translated, and copy the
> - * name into the buffer.
> - */
> - cnp->cn_pnbuf = uma_zalloc(namei_zone, M_WAITOK);
> - if (ndp->ni_segflg == UIO_SYSSPACE)
> - error = copystr(ndp->ni_dirp, cnp->cn_pnbuf, MAXPATHLEN,
> - >ni_pathlen);
> - else
> - error = copyinstr(ndp->ni_dirp, cnp->cn_pnbuf, MAXPATHLEN,
> - >ni_pathlen);
> -
> + error = namei_getpath(ndp);
> if (__predict_false(error != 0)) {
> - namei_cleanup_cnp(cnp);
But it used to be called in that case here.
> return (error);
> }
Jess
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
svn commit: r368375 - head/sys/kern
Author: mjg
Date: Sun Dec 6 04:59:24 2020
New Revision: 368375
URL: https://svnweb.freebsd.org/changeset/base/368375
Log:
vfs: factor buffer allocation/copyin out of namei
Modified:
head/sys/kern/vfs_lookup.c
Modified: head/sys/kern/vfs_lookup.c
==
--- head/sys/kern/vfs_lookup.c Sat Dec 5 22:04:30 2020(r368374)
+++ head/sys/kern/vfs_lookup.c Sun Dec 6 04:59:24 2020(r368375)
@@ -464,6 +464,43 @@ namei_setup(struct nameidata *ndp, struct vnode **dpp,
return (0);
}
+static int
+namei_getpath(struct nameidata *ndp)
+{
+ struct componentname *cnp;
+ int error;
+
+ cnp = >ni_cnd;
+
+ /*
+* Get a buffer for the name to be translated, and copy the
+* name into the buffer.
+*/
+ cnp->cn_pnbuf = uma_zalloc(namei_zone, M_WAITOK);
+ if (ndp->ni_segflg == UIO_SYSSPACE) {
+ error = copystr(ndp->ni_dirp, cnp->cn_pnbuf, MAXPATHLEN,
+ >ni_pathlen);
+ } else {
+ error = copyinstr(ndp->ni_dirp, cnp->cn_pnbuf, MAXPATHLEN,
+ >ni_pathlen);
+ }
+
+ if (__predict_false(error != 0)) {
+ return (error);
+ }
+
+ /*
+* Don't allow empty pathnames.
+*/
+ if (__predict_false(*cnp->cn_pnbuf == '\0')) {
+ namei_cleanup_cnp(cnp);
+ return (ENOENT);
+ }
+
+ cnp->cn_nameptr = cnp->cn_pnbuf;
+ return (0);
+}
+
/*
* Convert a pathname into a pointer to a locked vnode.
*
@@ -531,31 +568,11 @@ namei(struct nameidata *ndp)
ndp->ni_lcf = 0;
ndp->ni_vp = NULL;
- /*
-* Get a buffer for the name to be translated, and copy the
-* name into the buffer.
-*/
- cnp->cn_pnbuf = uma_zalloc(namei_zone, M_WAITOK);
- if (ndp->ni_segflg == UIO_SYSSPACE)
- error = copystr(ndp->ni_dirp, cnp->cn_pnbuf, MAXPATHLEN,
- >ni_pathlen);
- else
- error = copyinstr(ndp->ni_dirp, cnp->cn_pnbuf, MAXPATHLEN,
- >ni_pathlen);
-
+ error = namei_getpath(ndp);
if (__predict_false(error != 0)) {
- namei_cleanup_cnp(cnp);
return (error);
}
- /*
-* Don't allow empty pathnames.
-*/
- if (__predict_false(*cnp->cn_pnbuf == '\0')) {
- namei_cleanup_cnp(cnp);
- return (ENOENT);
- }
-
#ifdef KTRACE
if (KTRPOINT(td, KTR_NAMEI)) {
KASSERT(cnp->cn_thread == curthread,
@@ -563,8 +580,6 @@ namei(struct nameidata *ndp)
ktrnamei(cnp->cn_pnbuf);
}
#endif
-
- cnp->cn_nameptr = cnp->cn_pnbuf;
/*
* First try looking up the target without locking any vnodes.
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
