> I also had a couple of random thoughts (bear in mind I am not a linux
> kernel hacker):
>
> I personally don't tend to like static limits on number of array
> elements (since if you decided that the limit should be foo, I might
> have a good reason to want 2*foo different formats):
>
> spin_lock(&misc_binfmts_lock);
> if (misc_binfmts_last == MAX_BINFMT_MISC_ENTRIES) {
> kfree(ek);
> spin_unlock(&misc_binfmts_lock);
> return -ENOSPC;
> }
>
> misc_binfmts[++misc_binfmts_last]=ek;
>
> spin_unlock(&misc_binfmts_lock);
> return 1;
>
> Writing it as a linked list would avoid the problem.
Yes, I'll think about this.
> In load_misc_binary, I suspect you may want to build the arguments
> (using copy_string) outside of the spin lock for misc_binfmts_lock,
> particularly on a SMP machine if copy_string blocks awaiting for
> memory. Whether or not you want to go to separate spin locks for each
> element might be something to think about as the kernel goes away from
> one giant spin lock around the kernel.
I had the problem, that I rely on misc_binfmts[fmt] beeing correct _after_
the copy_strings() call (for the open_namei one). Therefore I need to
hold the lock this long time (I dont like this, too).
> Not knowing the guts of the Linux kernel, I assume that there is
> probably a function to do the strlen code you open code in
> binfmt_misc_sysctl:
>
> /* strlen of interpreter */
> while (!(res=get_user(c, eu.interpreter+len)) && (c!=0))
> len++;
I did not found such a function... (problem is, the string is in user-space!)
But I found a cleaner solution to this (and an other) problem: Now I use
different structs for the sysctl and the internal data-management (see
NEW patch on www.anatom.uni-tuebingen.de/~richi/binfmt_misc.html)
Richard.