Mailing List Archive: Linux: Kernel

[PATCH] export kernel call get_task_comm().

Index | Next | Previous | View Threaded

james_p_freyensee at linux

Apr 22, 2011, 3:26 PM

Post #1 of 10 (302 views)
Permalink

[PATCH] export kernel call get_task_comm().

From: J Freyensee <james_p_freyensee [at] linux>

This allows drivers who call this function to be compiled modularly.
Otherwise, a driver who is interested in this type of functionality
has to implement their own get_task_comm() call, causing code
duplication in the Linux source tree.

Signed-off-by: J Freyensee <james_p_freyensee [at] linux>
---
fs/exec.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 8328beb..e1ac338 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1004,6 +1004,7 @@ char *get_task_comm(char *buf, struct task_struct *tsk)
task_unlock(tsk);
return buf;
}
+EXPORT_SYMBOL_GPL(get_task_comm);

void set_task_comm(struct task_struct *tsk, char *buf)
{
--
1.7.2.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

james_p_freyensee at linux

Apr 22, 2011, 3:35 PM

Post #2 of 10 (292 views)
Permalink

[PATCH] export kernel call get_task_comm(). [In reply to]

rientjes at google

Apr 22, 2011, 3:35 PM

Post #3 of 10 (290 views)
Permalink

Re: [PATCH] export kernel call get_task_comm(). [In reply to]

On Fri, 22 Apr 2011, james_p_freyensee [at] linux wrote:

> From: J Freyensee <james_p_freyensee [at] linux>
>
> This allows drivers who call this function to be compiled modularly.
> Otherwise, a driver who is interested in this type of functionality
> has to implement their own get_task_comm() call, causing code
> duplication in the Linux source tree.
>
> Signed-off-by: J Freyensee <james_p_freyensee [at] linux>

Acked-by: David Rientjes <rientjes [at] google>

I still suggest that we implement finer-grained protection for tsk->comm
through get_task_comm(), though, because it's going to be difficult to
know whether task_lock(tsk) is held in all contexts we'll want to call it;
task_lock(tsk) is used to protect many members of task_struct.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

gregkh at suse

Apr 22, 2011, 3:43 PM

Post #4 of 10 (291 views)
Permalink

Re: [PATCH] export kernel call get_task_comm(). [In reply to]

On Fri, Apr 22, 2011 at 03:35:44PM -0700, james_p_freyensee [at] linux wrote:
> From: J Freyensee <james_p_freyensee [at] linux>
>
> This allows drivers who call this function to be compiled modularly.
> Otherwise, a driver who is interested in this type of functionality
> has to implement their own get_task_comm() call, causing code
> duplication in the Linux source tree.
>
> Signed-off-by: J Freyensee <james_p_freyensee [at] linux>

I think the goal is for the cleanup to happen now, to justify the
addition of the exported symbol. Without that, there is no need to
export the symbol now at all, as who knows when your driver will be
accepted.

Or, just wait and make it part of your driver patch series, like you did
before, no need to get it accepted now, right?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

james_p_freyensee at linux

Apr 22, 2011, 3:43 PM

Post #5 of 10 (295 views)
Permalink

Re: [PATCH] export kernel call get_task_comm(). [In reply to]

On Fri, 2011-04-22 at 15:35 -0700, David Rientjes wrote:
> On Fri, 22 Apr 2011, james_p_freyensee [at] linux wrote:
>
> > From: J Freyensee <james_p_freyensee [at] linux>
> >
> > This allows drivers who call this function to be compiled modularly.
> > Otherwise, a driver who is interested in this type of functionality
> > has to implement their own get_task_comm() call, causing code
> > duplication in the Linux source tree.
> >
> > Signed-off-by: J Freyensee <james_p_freyensee [at] linux>
>
> Acked-by: David Rientjes <rientjes [at] google>
>
> I still suggest that we implement finer-grained protection for tsk->comm
> through get_task_comm(), though, because it's going to be difficult to
> know whether task_lock(tsk) is held in all contexts we'll want to call it;
> task_lock(tsk) is used to protect many members of task_struct.

Okay, but how about accepting this as step 1, then investigate a finer
grained lock structure as step 2?

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

james_p_freyensee at linux

Apr 22, 2011, 3:59 PM

Post #6 of 10 (289 views)
Permalink

Re: [PATCH] export kernel call get_task_comm(). [In reply to]

On Fri, 2011-04-22 at 15:43 -0700, Greg KH wrote:
> On Fri, Apr 22, 2011 at 03:35:44PM -0700, james_p_freyensee [at] linux wrote:
> > From: J Freyensee <james_p_freyensee [at] linux>
> >
> > This allows drivers who call this function to be compiled modularly.
> > Otherwise, a driver who is interested in this type of functionality
> > has to implement their own get_task_comm() call, causing code
> > duplication in the Linux source tree.
> >
> > Signed-off-by: J Freyensee <james_p_freyensee [at] linux>
>
> I think the goal is for the cleanup to happen now, to justify the
> addition of the exported symbol. Without that, there is no need to
> export the symbol now at all, as who knows when your driver will be
> accepted.
>
> Or, just wait and make it part of your driver patch series, like you did
> before, no need to get it accepted now, right?
>

Well, at some point a few people like Alan Cox and Arjan VdV would like
to see this work on it's way to Linus's tree.

I'll do whatever is best and easiest for you and will bring a close to
my submission attempts. I can also just go into the Kconfig where the
pti driver is configured and just make the selection bool, yes or no,
and not make it an option to compile this modularly. Then I'll drop
this patch all together. This is the whole reason why I'm making this
change. I don't have to have the pti driver as a module, just more
convenient. And within the fs/exec.c it states reads to 'current->comm'
without a lock is safe.

> thanks,
>
> greg k-h

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

gregkh at suse

Apr 22, 2011, 4:04 PM

Post #7 of 10 (289 views)
Permalink

Re: [PATCH] export kernel call get_task_comm(). [In reply to]

On Fri, Apr 22, 2011 at 03:59:42PM -0700, J Freyensee wrote:
> On Fri, 2011-04-22 at 15:43 -0700, Greg KH wrote:
> > On Fri, Apr 22, 2011 at 03:35:44PM -0700, james_p_freyensee [at] linux wrote:
> > > From: J Freyensee <james_p_freyensee [at] linux>
> > >
> > > This allows drivers who call this function to be compiled modularly.
> > > Otherwise, a driver who is interested in this type of functionality
> > > has to implement their own get_task_comm() call, causing code
> > > duplication in the Linux source tree.
> > >
> > > Signed-off-by: J Freyensee <james_p_freyensee [at] linux>
> >
> > I think the goal is for the cleanup to happen now, to justify the
> > addition of the exported symbol. Without that, there is no need to
> > export the symbol now at all, as who knows when your driver will be
> > accepted.
> >
> > Or, just wait and make it part of your driver patch series, like you did
> > before, no need to get it accepted now, right?
> >
>
> Well, at some point a few people like Alan Cox and Arjan VdV would like
> to see this work on it's way to Linus's tree.

That's because that is the policy of your distro you are working with,
which has nothing to do with the kernel developers.

Again, if you get your driver accepted, I have no objection to this
export at all. Just take the time and get your driver merged, it's that
simple.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

james_p_freyensee at linux

Apr 22, 2011, 4:08 PM

Post #8 of 10 (288 views)
Permalink

Re: [PATCH] export kernel call get_task_comm(). [In reply to]

On Fri, 2011-04-22 at 16:04 -0700, Greg KH wrote:
> On Fri, Apr 22, 2011 at 03:59:42PM -0700, J Freyensee wrote:
> > On Fri, 2011-04-22 at 15:43 -0700, Greg KH wrote:
> > > On Fri, Apr 22, 2011 at 03:35:44PM -0700, james_p_freyensee [at] linux wrote:
> > > > From: J Freyensee <james_p_freyensee [at] linux>
> > > >
> > > > This allows drivers who call this function to be compiled modularly.
> > > > Otherwise, a driver who is interested in this type of functionality
> > > > has to implement their own get_task_comm() call, causing code
> > > > duplication in the Linux source tree.
> > > >
> > > > Signed-off-by: J Freyensee <james_p_freyensee [at] linux>
> > >
> > > I think the goal is for the cleanup to happen now, to justify the
> > > addition of the exported symbol. Without that, there is no need to
> > > export the symbol now at all, as who knows when your driver will be
> > > accepted.
> > >
> > > Or, just wait and make it part of your driver patch series, like you did
> > > before, no need to get it accepted now, right?
> > >
> >
> > Well, at some point a few people like Alan Cox and Arjan VdV would like
> > to see this work on it's way to Linus's tree.
>
> That's because that is the policy of your distro you are working with,
> which has nothing to do with the kernel developers.
>
> Again, if you get your driver accepted, I have no objection to this
> export at all. Just take the time and get your driver merged, it's that
> simple.

So I guess the best route is for me to make this patch with my driver
then? I'm ready to re-submit those drivers again; I cleaned up all the
style issues pointed out by Randy.

>
> thanks,
>
> greg k-h

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

gregkh at suse

Apr 22, 2011, 4:17 PM

Post #9 of 10 (290 views)
Permalink

Re: [PATCH] export kernel call get_task_comm(). [In reply to]

On Fri, Apr 22, 2011 at 04:08:16PM -0700, J Freyensee wrote:
> On Fri, 2011-04-22 at 16:04 -0700, Greg KH wrote:
> > On Fri, Apr 22, 2011 at 03:59:42PM -0700, J Freyensee wrote:
> > > On Fri, 2011-04-22 at 15:43 -0700, Greg KH wrote:
> > > > On Fri, Apr 22, 2011 at 03:35:44PM -0700, james_p_freyensee [at] linux wrote:
> > > > > From: J Freyensee <james_p_freyensee [at] linux>
> > > > >
> > > > > This allows drivers who call this function to be compiled modularly.
> > > > > Otherwise, a driver who is interested in this type of functionality
> > > > > has to implement their own get_task_comm() call, causing code
> > > > > duplication in the Linux source tree.
> > > > >
> > > > > Signed-off-by: J Freyensee <james_p_freyensee [at] linux>
> > > >
> > > > I think the goal is for the cleanup to happen now, to justify the
> > > > addition of the exported symbol. Without that, there is no need to
> > > > export the symbol now at all, as who knows when your driver will be
> > > > accepted.
> > > >
> > > > Or, just wait and make it part of your driver patch series, like you did
> > > > before, no need to get it accepted now, right?
> > > >
> > >
> > > Well, at some point a few people like Alan Cox and Arjan VdV would like
> > > to see this work on it's way to Linus's tree.
> >
> > That's because that is the policy of your distro you are working with,
> > which has nothing to do with the kernel developers.
> >
> > Again, if you get your driver accepted, I have no objection to this
> > export at all. Just take the time and get your driver merged, it's that
> > simple.
>
> So I guess the best route is for me to make this patch with my driver
> then? I'm ready to re-submit those drivers again; I cleaned up all the
> style issues pointed out by Randy.

Yes, make it part of that patch series.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

rientjes at google

Apr 22, 2011, 4:19 PM

Post #10 of 10 (292 views)
Permalink

Re: [PATCH] export kernel call get_task_comm(). [In reply to]

On Fri, 22 Apr 2011, J Freyensee wrote:

> I'll do whatever is best and easiest for you and will bring a close to
> my submission attempts. I can also just go into the Kconfig where the
> pti driver is configured and just make the selection bool, yes or no,
> and not make it an option to compile this modularly. Then I'll drop
> this patch all together. This is the whole reason why I'm making this
> change. I don't have to have the pti driver as a module, just more
> convenient. And within the fs/exec.c it states reads to 'current->comm'
> without a lock is safe.
>

It's safe to read current->comm without holding task_lock(current), but it
may be corrupted by a concurrent write via /proc/pid-of-current/comm,
which could result in garbage where you'd expect the name of the thread.
That doesn't sound very clean to me and adds more incentive to implement
some finer-grained protection like a seqlock specifically for tsk->comm.

If /proc/pid/comm is really that important and we can't get away with the
long-standing prctl(PR_SET_NAME), then you need to protect the string
somehow and I'm quite surprised this wasn't required before writing other
threads' comm was allowed.

If you can get away with task_lock(current) in your driver, then great,
export the symbol and use it, but we have hundreds of racy reads to a
thread's comm all over the kernel.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Index | Next | Previous | View Threaded

Interested in having your list archived? Contact Gossamer Threads