[Goodies-dev] xfce4-sensors-plugin: hddtemp not setuid message

[Mariano Besio]

2008/3/1, Fabian Nowak <timystery at arcor.de>:
> Hey Mariano,
>
>
>  > > No, it looks for being executable in general but returning invalid
>  > >  stuff; so it comes to the conclusion that the SMARt sensor cannot be
>  > >  polled due to missing permissions.
>  > >
>  >
>  > Oh, yes now I see which is the idea. So that was why changing that
>  > value worked for me. Sorry for that, my mistake.
>
>
> No problem at all.
>
>
>  > >
>  > >  Anyway, did some changes in svn, you might want to check them and
>  > >  whether it's working better. If not, it might be nice to give me the
>  > >  return codes and output to stdout and stderr of running hddtemp on valid
>  > >  and invalid devices both with sudo/setuid root and without.
>  >
>  > I'm running hddtemp version 0.3-beta15. I've got this outputs:
>  >
>  > With setuid
>  > $ ll /usr/sbin/hddtemp ; hddtemp /dev/sda ; echo "\$? = "$?
>  > -rwsr-xr-x 1 root root 26716 2008-02-17 15:52 /usr/sbin/hddtemp
>  > /dev/sda: WDC WD1600JS-00MHB0: 56°C
>  > $? = 0
>  >
>  > $ ll /usr/sbin/hddtemp ; hddtemp /dev/fd ; echo "\$? = "$?
>  > -rwsr-xr-x 1 root root 26716 2008-02-17 15:52 /usr/sbin/hddtemp
>  > ERROR: /dev/fd: can't determine bus type (or this bus type is unknown)
>  > $? = 1
>  >
>  > $ ll /usr/sbin/hddtemp ; hddtemp /dev/null ; echo "\$? = "$?
>  > -rwsr-xr-x 1 root root 26716 2008-02-17 15:52 /usr/sbin/hddtemp
>  > ERROR: /dev/null: can't determine bus type (or this bus type is unknown)
>  > $? = 1
>  >
>  > $ ll /usr/sbin/hddtemp ; hddtemp /dev/nodevice ; echo "\$? = "$?
>  > -rwsr-xr-x 1 root root 26716 2008-02-17 15:52 /usr/sbin/hddtemp
>  > /dev/nodevice: open: No existe el fichero o el directorio
>  >
>  > $? = 1
>  > (this has the system locale output, translation: "No file or
>  > directory". I know this case should never happen as names are fetched
>  > form sys, just to be more informative)
>  >
>  > Without setuid
>  > $ ll /usr/sbin/hddtemp ; hddtemp /dev/sda ; echo "\$? = "$?
>  > -rwxr-xr-x 1 root root 26716 2008-02-17 15:52 /usr/sbin/hddtemp
>  > /dev/sda: open: Permiso denegado
>  >
>  > $? = 1
>  > (again the locale, translation: "Permission denied")
>  >
>  > $ ll /usr/sbin/hddtemp ; hddtemp /dev/fd ; echo "\$? = "$?
>  > -rwxr-xr-x 1 root root 26716 2008-02-17 15:52 /usr/sbin/hddtemp
>  > ERROR: /dev/fd: can't determine bus type (or this bus type is unknown)
>  > $? = 1
>  >
>  > $ ll /usr/sbin/hddtemp ; hddtemp /dev/null ; echo "\$? = "$?
>  > -rwxr-xr-x 1 root root 26716 2008-02-17 15:52 /usr/sbin/hddtemp
>  > ERROR: /dev/null: can't determine bus type (or this bus type is unknown)
>  > $? = 1
>  >
>  > $ ll /usr/sbin/hddtemp ; hddtemp /dev/nodevice ; echo "\$? = "$?
>  > -rwxr-xr-x 1 root root 26716 2008-02-17 15:52 /usr/sbin/hddtemp
>  > /dev/nodevice: open: No existe el fichero o el directorio
>  >
>  > $? = 1
>  > (translation: "No file or directory")
>
>
> Thanks for the error codes - and your understanding of how bad it is to
>  be used.
>
>
>  >
>  > Doing "grep exit *.c" on hddtemp code in
>  > http://download.savannah.nongnu.org/releases/hddtemp/ I've found that
>  > the author is using (or planning to use) these error codes: -2, -1, 1,
>  > 2, 3. He seems to use mostly 1. But as you had already written in some
>  > comments, those checks are version dependent.
>
>  :)
>
>  > The changes in svn revision 4012 look fine. Don't misunderstand me, my
>  > doubt was more like a theoretical one. The svn version seems to be
>  > working OK, but IMHO the strings comparison for devices seems a bit
>  > hardwired. I don't want to be pedantic, but perhaps in this case you
>  > may want to look at the stat function instead of access, although the
>  > bitwise operations make code less maintainable. This weekend I'll try
>  > to see if the stat solution is viable to avoid this cases.
>
>
> Yeah, this night I've also been wondering why not simply checking for
>  setuid bit would work; but you know, there are rot users under X around
>  this world... (some of  them administrate our university servers...) and
>  again, checking the way I do just enables "understanding" or
>  "justifying" the return codes  from the point of view of the plugin with
>  regard to hddtemp's behaviour.
>
>  For getting the device names, I don't think fstat is appropriate.
>
>  Anyway, I'd really appreciate your investigations as I'm beginning to
>  move out of my current flat for this month and won't have plenty of
>  time.
>
>
>  >
>  > Hope I haven't been too harsh, as you've seen in the locale, English
>  > is not my native language.
>
>
> No, it's all perfect; and you don't need to translate any German,
>  Spanish, Italian, Portuguese, French for me, I can at least read and
>  assemble them ;)
>
>
>  > Thanks for the answers and your time. Regards,
>
>
> Again, you're welcome, thanks in return, cheers
>
>  *gonnapaintaflatnow* Fabian
>

Hi Fabien,

I'm sorry I couldn't give you some feedback some days ago. University,
among other things, kept me busy. Anyway I managed to make some
refactoring in the patch I'm attaching. I've made some testing on it -
even running it from root (I've never used X with root before :S) and
seems to work. Have a look at it and take whatever you find useful (if
something :D) from it.

These are the the changes I made:
- Although I couldn't find a way to get rid of strncmp's to get the
devices, I changed the string comparison to get only those which I
think that could have a sensor ("sd." and "hd.").
- Separated check for error of permission and program. First check for
suid or root, then check for any other errors. I hope that the error
messages are clear enough.

By the way, today I received a mail for a bug/enhancement
[Goodies-dev] [Bug 3953]. I tried to replicate that on my system but I
wasn't successful (I'm on Debian not Ubuntu, but versions are the
same), even renaming hddtemp binary. IMHO seems to be a problem with
Xubuntu's dependencies as xfce-sensors package does not depends on
hddtemp package. Anyway, the bug report doesn't specifies plugin
version.

Thanks for your time, and best wishes on your new flat. Regards,

Mariano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: hddtemp.c.patch
Type: text/x-patch
Size: 5370 bytes
Desc: not available
URL: <http://mail.xfce.org/pipermail/goodies-dev/attachments/20080320/f00be1c3/attachment.bin>