Skip to contentSkip to conversation

export kernel call get_task_comm().

PATCH In review [email protected] 3 participants 7 messages Apr 22, 2011

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 export kernel call get_task_comm().

From: J Freyensee <[email protected]>

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.

---
 fs/exec.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
fs/exec.c +1 -0
index 8328beb..e1ac338 100644
@@ -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)
 {
Signature · 1 line
1.7.2.3
#1 lore raw
Quoted 8 lines · On Fri, Apr 22, 2011 at 03:35:44PM -0700, [email protected]…

From: J Freyensee <[email protected]>

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.

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

#2 lore raw
Quoted 10 lines · Greg KH
Quoted 8 lines · On Fri, Apr 22, 2011 at 03:35:44PM -0700, [email protected]…

From: J Freyensee <[email protected]>

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.

Quoted 8 lines

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.

Quoted 3 lines

thanks,

greg k-h

#3 lore raw
Quoted 16 lines · J Freyensee
Quoted 15 lines · Greg KH
Quoted 8 lines · On Fri, Apr 22, 2011 at 03:35:44PM -0700, [email protected]…

From: J Freyensee <[email protected]>

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.

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.

Quoted 6 lines
Quoted 3 lines

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

#4 lore raw
Quoted 24 lines · Greg KH
Quoted 16 lines · J Freyensee
Quoted 15 lines · Greg KH
Quoted 8 lines · On Fri, Apr 22, 2011 at 03:35:44PM -0700, [email protected]…

From: J Freyensee <[email protected]>

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.

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.

Quoted 6 lines
Quoted 3 lines

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.

Quoted 6 lines

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.

Quoted 4 lines

thanks,

greg k-h

#5 lore raw
Quoted 28 lines · J Freyensee
Quoted 27 lines · Greg KH
Quoted 16 lines · J Freyensee
Quoted 15 lines · Greg KH
Quoted 8 lines · On Fri, Apr 22, 2011 at 03:35:44PM -0700, [email protected]…

From: J Freyensee <[email protected]>

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.

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.

Quoted 6 lines
Quoted 3 lines

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.

Quoted 7 lines
Quoted 3 lines

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

#6 lore raw
In reply to J Freyensee
Quoted 9 lines · J Freyensee

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.