Bug 157871 - [linprocfs] [patch] linprocfs: cmdline/environ for kernel-only processes returns EFAULT instead of empty string
Summary: [linprocfs] [patch] linprocfs: cmdline/environ for kernel-only processes retu...
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: Unspecified
Hardware: Any Any
: Normal Affects Only Me
Assignee: Sergey Kandaurov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-06-14 11:30 UTC by Petr Salinger
Modified: 2011-07-01 11:30 UTC (History)
0 users

See Also:


Attachments
linprocfs.diff (943 bytes, patch)
2011-06-15 08:44 UTC, Sergey Kandaurov
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Petr Salinger 2011-06-14 11:30:11 UTC
Between 8.1 and 8.2 have been in linprocfs added support for $PID/environ and changed implementation of $PID/cmdline. Now it for kernel-only processes returns EFAULT instead of empty string. It breaks at least "pstree -a".

See Debian GNU/kFreeBSD bug #630104
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=630104

Fix: 

--- a/sys/compat/linprocfs/linprocfs.c
+++ b/sys/compat/linprocfs/linprocfs.c
@@ -1044,6 +1044,10 @@
                PROC_UNLOCK(p);
                return (ret);
        }
+       if (p->p_flag & P_KTHREAD) {
+               PROC_UNLOCK(p);
+               return (0);
+       }
        if (p->p_args != NULL) {
                sbuf_bcpy(sb, p->p_args->ar_args, p->p_args->ar_length);
                PROC_UNLOCK(p);
@@ -1068,6 +1072,10 @@
                PROC_UNLOCK(p);
                return (ret);
        }
+       if (p->p_flag & P_KTHREAD) {
+               PROC_UNLOCK(p);
+               return (0);
+       }
        PROC_UNLOCK(p);

        ret = linprocfs_doargv(td, p, sb, ps_string_env);
Comment 1 Mark Linimon freebsd_committer freebsd_triage 2011-06-14 15:33:20 UTC
Responsible Changed
From-To: freebsd-bugs->freebsd-emulation

Over to maintainer(s).
Comment 2 Sergey Kandaurov freebsd_committer freebsd_triage 2011-06-14 16:14:31 UTC
Responsible Changed
From-To: freebsd-emulation->pluknet

I'll take it.
Comment 3 Sergey Kandaurov freebsd_committer freebsd_triage 2011-06-15 08:44:16 UTC
Hi.

Checking for P_KTHREAD doesn't work for kernel process 0, so it can't serve
as a kernel process indicator. It's not obvious how to find a way to separate
processes with such present usermode attributes as environ and cmdline.
[Kernel threads/procs have cmdline however it doesn't work in linux /proc]
Please try the attached patch and report your results.

-- 
wbr,
pluknet
Comment 4 Petr Salinger 2011-06-15 18:53:18 UTC
Hi.

> Checking for P_KTHREAD doesn't work for kernel process 0, so it can't serve
> as a kernel process indicator. It's not obvious how to find a way to separate
> processes with such present usermode attributes as environ and cmdline.
> [Kernel threads/procs have cmdline however it doesn't work in linux /proc]
> Please try the attached patch and report your results.

It works correctly, thanks.

Petr
Comment 5 dfilter service freebsd_committer freebsd_triage 2011-06-17 08:31:11 UTC
Author: pluknet
Date: Fri Jun 17 07:30:56 2011
New Revision: 223182
URL: http://svn.freebsd.org/changeset/base/223182

Log:
  Return empty cmdline/environ string for processes with kernel address
  space. This is consistent with the behavior in linux.
  
  PR:		kern/157871
  Reported by:	Petr Salinger <Petr Salinger att seznam cz>
  Verified on:	GNU/kFreeBSD debian 8.2-1-amd64 (by reporter)
  Reviewed by:	kib (some time ago)
  MFC after:	2 weeks

Modified:
  head/sys/compat/linprocfs/linprocfs.c

Modified: head/sys/compat/linprocfs/linprocfs.c
==============================================================================
--- head/sys/compat/linprocfs/linprocfs.c	Fri Jun 17 07:07:26 2011	(r223181)
+++ head/sys/compat/linprocfs/linprocfs.c	Fri Jun 17 07:30:56 2011	(r223182)
@@ -1049,6 +1049,15 @@ linprocfs_doproccmdline(PFS_FILL_ARGS)
 		PROC_UNLOCK(p);
 		return (ret);
 	}
+
+	/*
+	 * Mimic linux behavior and pass only processes with usermode
+	 * address space as valid.  Return zero silently otherwize.
+	 */
+	if (p->p_vmspace == &vmspace0) {
+		PROC_UNLOCK(p);
+		return (0);
+	}
 	if (p->p_args != NULL) {
 		sbuf_bcpy(sb, p->p_args->ar_args, p->p_args->ar_length);
 		PROC_UNLOCK(p);
@@ -1073,6 +1082,15 @@ linprocfs_doprocenviron(PFS_FILL_ARGS)
 		PROC_UNLOCK(p);
 		return (ret);
 	}
+
+	/*
+	 * Mimic linux behavior and pass only processes with usermode
+	 * address space as valid.  Return zero silently otherwize.
+	 */
+	if (p->p_vmspace == &vmspace0) {
+		PROC_UNLOCK(p);
+		return (0);
+	}
 	PROC_UNLOCK(p);
 
 	ret = linprocfs_doargv(td, p, sb, ps_string_env);
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 6 Sergey Kandaurov freebsd_committer freebsd_triage 2011-06-17 08:31:59 UTC
State Changed
From-To: open->patched

Committed to HEAD, thanks!
Comment 7 dfilter service freebsd_committer freebsd_triage 2011-07-01 11:26:02 UTC
Author: pluknet
Date: Fri Jul  1 10:25:48 2011
New Revision: 223707
URL: http://svn.freebsd.org/changeset/base/223707

Log:
  MFC r223182:
  
   Return empty cmdline/environ string for processes with kernel address
   space. This is consistent with the behavior in linux.
  
  PR:		kern/157871

Modified:
  stable/8/sys/compat/linprocfs/linprocfs.c
Directory Properties:
  stable/8/sys/   (props changed)
  stable/8/sys/amd64/include/xen/   (props changed)
  stable/8/sys/cddl/contrib/opensolaris/   (props changed)
  stable/8/sys/contrib/dev/acpica/   (props changed)
  stable/8/sys/contrib/pf/   (props changed)

Modified: stable/8/sys/compat/linprocfs/linprocfs.c
==============================================================================
--- stable/8/sys/compat/linprocfs/linprocfs.c	Fri Jul  1 08:27:03 2011	(r223706)
+++ stable/8/sys/compat/linprocfs/linprocfs.c	Fri Jul  1 10:25:48 2011	(r223707)
@@ -1046,6 +1046,15 @@ linprocfs_doproccmdline(PFS_FILL_ARGS)
 		PROC_UNLOCK(p);
 		return (ret);
 	}
+
+	/*
+	 * Mimic linux behavior and pass only processes with usermode
+	 * address space as valid.  Return zero silently otherwize.
+	 */
+	if (p->p_vmspace == &vmspace0) {
+		PROC_UNLOCK(p);
+		return (0);
+	}
 	if (p->p_args != NULL) {
 		sbuf_bcpy(sb, p->p_args->ar_args, p->p_args->ar_length);
 		PROC_UNLOCK(p);
@@ -1070,6 +1079,15 @@ linprocfs_doprocenviron(PFS_FILL_ARGS)
 		PROC_UNLOCK(p);
 		return (ret);
 	}
+
+	/*
+	 * Mimic linux behavior and pass only processes with usermode
+	 * address space as valid.  Return zero silently otherwize.
+	 */
+	if (p->p_vmspace == &vmspace0) {
+		PROC_UNLOCK(p);
+		return (0);
+	}
 	PROC_UNLOCK(p);
 
 	ret = linprocfs_doargv(td, p, sb, ps_string_env);
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 8 Sergey Kandaurov freebsd_committer freebsd_triage 2011-07-01 11:26:49 UTC
State Changed
From-To: patched->closed

Merged to stable/8.