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.
Signed-off-by: J Freyensee <[email protected]>
---
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
On Fri, Apr 22, 2011 at 03:35:44PM -0700, [email protected] wrote:
> 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.
>
> Signed-off-by: J Freyensee <[email protected]>
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
On Fri, 2011-04-22 at 15:43 -0700, Greg KH wrote:
> On Fri, Apr 22, 2011 at 03:35:44PM -0700, [email protected] wrote:
> > 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.
> >
> > Signed-off-by: J Freyensee <[email protected]>
>
> 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
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, [email protected] wrote:
> > > 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.
> > >
> > > Signed-off-by: J Freyensee <[email protected]>
> >
> > 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
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, [email protected] wrote:
> > > > 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.
> > > >
> > > > Signed-off-by: J Freyensee <[email protected]>
> > >
> > > 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
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, [email protected] wrote:
> > > > > 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.
> > > > >
> > > > > Signed-off-by: J Freyensee <[email protected]>
> > > >
> > > > 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
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.