linux-kernel - Re: [PATCH 2/4] staging/lustre/obdclass: read jobid from proc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+a=Yy6rL6CV3THZ4Mr4oHqVDiejr++r9Oa4kro4Y_CCbFEfEQ@mail.gmail.com>
Date:	Wed, 30 Oct 2013 21:31:24 +0800
From:	Peng Tao <bergwolf@...il.com>
To:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Andreas Dilger <andreas.dilger@...el.com>
Subject: Re: [PATCH 2/4] staging/lustre/obdclass: read jobid from proc

On Wed, Oct 30, 2013 at 9:21 PM, Greg Kroah-Hartman
<gregkh@...uxfoundation.org> wrote:
> On Wed, Oct 30, 2013 at 07:30:34PM +0800, Peng Tao wrote:
>> so that we can get rid of cfs_get_environ() that needs access_process_vm() that
>> is a core mm function and is not available on some architectures.
>>
>> Reviewed-by: Niu Yawei <yawei.niu@...el.com>
>> Signed-off-by: Peng Tao <bergwolf@...il.com>
>> Signed-off-by: Andreas Dilger <andreas.dilger@...el.com>
>> ---
>>  drivers/staging/lustre/lustre/obdclass/class_obd.c |  119 +++++++++++++++++++-
>>  1 file changed, 115 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/staging/lustre/lustre/obdclass/class_obd.c b/drivers/staging/lustre/lustre/obdclass/class_obd.c
>> index b1024a6..99ce863 100644
>> --- a/drivers/staging/lustre/lustre/obdclass/class_obd.c
>> +++ b/drivers/staging/lustre/lustre/obdclass/class_obd.c
>> @@ -96,8 +96,117 @@ EXPORT_SYMBOL(obd_dirty_transit_pages);
>>  char obd_jobid_var[JOBSTATS_JOBID_VAR_MAX_LEN + 1] = JOBSTATS_DISABLE;
>>  EXPORT_SYMBOL(obd_jobid_var);
>>
>> -/* Get jobid of current process by reading the environment variable
>> - * stored in between the "env_start" & "env_end" of task struct.
>> +static char *self_environ_file = "/proc/self/environ";
>
> Heh, no, that's not ok at all.
>
> This is a _huge_ sign that you are doing something wrong in your driver
> if you need something that isn't exported, or that you have to dig out
> of proc.
>
Lustre has a functionality that lets applications specify a global
jobid by setting the same environment variable across the cluster and
thus server can trace IO that is issued by different job. Currently it
is implemented by having a private version of cfs_access_process_vm
because access_process_vm is not exported. The patch intends to change
it to reading from proc. If we cannot dig it out from proc either, how
can we keep the functionality then? Please advise.

Thanks,
Tao

> Sorry, I can't take this, please fix the underlying problems that would
> even think that you need access to the environment from within a kernel
> driver.
>
> greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/