On 04/07/10 13:38, Brian Cameron wrote:

Garrett/Mike/Albert:

I have attached an updated 1-pager and also updated the 1-pager in the
case directory. I have also attached a new patch which implements the
following changes for review:

This resolves my concerns about processes running as root inadvertently
doing bad things or being tricked into doing bad things.

It's strange that the Xsession logic respects $AUDIODEV but the
PostSession logic does not.  That seems unlikely to produce the correct
results if $AUDIODEV is ever set to something other than /dev/audio.  Of
course it's not easy to get hold of the session's $AUDIODEV in the
PostSession context.  You could do something like iterate over all of
the stored audio setting files for this machine, but I wonder whether
this is just more trouble than it's worth for the intended use case and
you might reasonably just wire both scripts to operate on /dev/audio.
(Subject to the check that that device is owned by this session's user,
of course.)  I don't feel strongly either way; my main concern was the
running-as-root issue, and that's been addressed.

Mike.
--
[email protected]


1) In the PostSession script the calls to mkdir and
"audioctl save-controls" are now called with
"/usr/binsu - $USER -c (cmd)" so that the commands which create
directories and files are run as the user.
2) The code to load the audio settings is now done in the
/etc/gdm/Xsession script after the user's $HOME/.profile is sourced
to ensure that any $AUDIODEV settings are honored on login.
3) The PostSession script and the Xsession script now call
"/usr/bin/stat -L /dev/audio -c %U" to find out the owner of
the audio device, and will only load or save the data if the
user is actually the owner of the device. This ensures that
only the user who actually has access to the device is modifying
the audio device settings.
4) Now, as suggested by Albert, the command to get the device is:

/usr/bin/audioctl show-device | /usr/bin/awk '/^ *Name /{ print $3; }'

Rather than using grep and sed, so that is a bit simpler.

I believe this should resolve all the issues raised about this change.

If a contract is needed to use "audioctl show-device" as described in
this case, then that is okay with me. Yes, this change only needs to
access the are "Device" value, which Garrett indicates should be
stable.

Brian


On 04/ 7/10 10:01 AM, Garrett D'Amore wrote:
Mike, you raise good issues. I was not aware of the environment which
PreSession and PostSession took place. I'd like these issues resolved,
and am withdrawing my +1 until they are properly satisfied.

As far as the output from audioctl show-device goes, we can handle that
in one of two ways:

1) add a contract for the interfaces use. I can arrange for my mgmt to
sign off on it.

2) increase the commitment level for just that subcommand. I'd be ok
with raising it to Uncommitted.

Actually, looking at the output, I think they are just using the Device:
field from the command, which is simple and parseable enough that I'm
willing to agree that at least *that* portion of it will never change.
Its the other results that (HW Info, etc.) that are potentially subject
to change.

- Garrett

On 04/ 7/10 01:26 AM, Mike Oliver wrote:
On 4/6/2010 7:56 PM, Brian Cameron wrote:

I have submitted the following case as PSARC 2010/116 with a timeout
next Wednesday, April 14th.

This is a pretty trivial interface change, so I am not expecting it
will require much review. I worked out the details with Garrett
D'Amore, and he already reviewed the one-pager and thought it was ready
to submit.

You're proposing to modify the Default PreSession and PostSession
scripts,
right? Not a display-specific script?

PreSession and PostSession scripts are executed as root. Do these
scripts
arrange to execute 'audioctl' as the logged-in (and logging-out) user? I
don't see this happening in the patch.

Are any measures taken to defend against someone logging in as root (or
some other suitably-privileged user) while someone else is using the
audio device and therefore clobbering the settings of the user who
really owns the audio device? (I know root is a role by default. That
doesn't prevent people from converting it back to a normal login
account. Obviously this is a bigger issue if these scripts don't run
'audioctl' as $USER.)

PSARC 2009/626 says that the device name that is retrieved by
'audioctl show-device' is sensitive to $AUDIODEV, but PostSession (and
probably PreSession) has no knowledge of what $AUDIODEV was set to in
the user's session. Is it fair to say that this case will only attempt
to save and restore the settings of the default audio device, which is
not necessarily the device that the user session will use?

The 'audioctl show-device' output consumed by this case was declared as
"Not An Interface" in PSARC 2009/626. Has the Stability of that output
been changed since then?

Does GDM guarantee to set $HOME appropriately for these scripts? I know
it sets $USER, I can't find anything that guarantees $HOME.

Code review nit: using both 'grep' and 'sed' in a single pipeline is
rarely necessary.

Mike.



_______________________________________________
opensolaris-arc mailing list
[email protected]

Reply via email to