Re: [kernel] r7596 - in people/jurij/ipw3945-daemon: . debian

[Jurij Smakov <jurij@wooyd.org>]

On Wed, Oct 11, 2006 at 10:05:45AM +0200, Bastian Blank wrote:
> On Wed, Oct 11, 2006 at 06:22:35AM +0000, Jurij Smakov wrote:
> > Log:
> > Initial commit (totally untested).
> 
> I see it, please see my comments inside. (FYI, I operate such a card.)
> 
> > Added: people/jurij/ipw3945-daemon/debian/control
> > ==============================================================================
> > --- (empty file)
> > +++ people/jurij/ipw3945-daemon/debian/control	Wed Oct 11 06:22:30 2006
> > @@ -0,0 +1,20 @@
> > +Source: ipw3945-daemon
> > +Section: admin
> 
> Must be non-free/admin.

Correct, will fix.
 
> > +DAEMON_RUN='yes'
> > +DAEMON_OWNER='Debian-ipw3945d:Debian-ipw3945d'
> > +DAEMON_BINARY='/sbin/ipw3945d'
> > +DAEMON_PERM='744'
> 
> See the policy, non-executable is only allowed for suid/sgid bins.

The name of this variable is a bit misleading. It is actually the 
permission, which the entries in the /sys tree are going to be 
chmod'ed to, not the daemon itself.
 
> > +DAEMON_PID='/var/run/ipw3945d.pid'
> 
> It makes never sense to modify the pid file location.

It's a matter of taste, I guess. I only added that variable because 
pid file is mentioned more than once in the script. Once the variable 
is there, I don't think there is any harm to let local admin to modify 
it.

> > Added: people/jurij/ipw3945-daemon/debian/init
> > ==============================================================================
> > --- (empty file)
> > +++ people/jurij/ipw3945-daemon/debian/init	Wed Oct 11 06:22:30 2006
> > @@ -0,0 +1,55 @@
> > +#!/bin/sh
> > +
> > +DAEMON_RUN='yes'
> > +DAEMON_OWNER='Debian-ipw3945d:Debian-ipw3945d'
> > +DAEMON_BINARY='/sbin/ipw3945d'
> > +DAEMON_PERM='744'
> > +DAEMON_PID='/var/run/ipw3945d.pid'
> > +
> > +if [ -r /etc/default/ipw3945-daemon ]; then
> > +  . /etc/default/ipw3945-daemon
> > +fi
> > +
> > +start_daemon() {
> > +  cmd="$(echo /sys/bus/pci/drivers/ipw3945/*/cmd)"
> > +  if [ -z "${cmd}" ]; then
> > +    echo "Not starting regulatory daemon, ipw3945 driver not loaded."
> > +  else
> > +    echo -n "Starting ipw3945 regulatory daemon: ipw3945d"
> > +    # Fix permissions
> > +    chown ${DAEMON_OWN} ${cmd}
> > +    chmod ${DAEMON_PERM} ${cmd}
> 
> Why? It is neither suid nor sgid.

Again ${cmd} is not the daemon itself, but the relevant entries in 
/sys. I have just followed the installation instructions in 
README file, so if I chown/chmod stuff in /sys and run daemon as
this special user, I don't see why it would not work. I will test it 
tonight.

> > +    start-stop-daemon --start --quiet --chuid ${DAEMON_OWN}        \
> > +                      --exec ${DAEMON_BIN} --pidfile ${DAEMON_PID} \
> > +		      -- --pid-path ${DAEMON_PID}
> 
> This will not work in default config. The daemon needs to run as root.

That contradicts the information in README file.

> The deamon should be started and stopped by modprobe via install/remove
> calls like
> install ipw3945 /sbin/modprobe --ignore-install ipw3945; $script
> or via udev and not via init. It will die if ipw3945 is not loaded
> anyway.

On my machine ipw3945 driver is loaded automatically by udev, so I 
believe that nothing else needs to be done. And the init script will 
not fail if driver is not loaded for some reason at the time the 
script runs, because it checks for the existence of entries in /sys.
 
> Also we don't want it to run as root, which means we can't use the
> current sysfs communication. I have a patch for the driver and a preload
> lib for the daemon to fix that. Also we can simply put it in a chroot
> than.

It will not run as root.

Thanks a lot for your feedback!

-- 
Jurij Smakov                                           jurij@wooyd.org
Key: http://www.wooyd.org/pgpkey/                      KeyID: C99E03CC