Re: [systemd-devel] [PATCH] Split sysctl 50-default.conf setting file

2013-12-02 Thread Kay Sievers
On Tue, Dec 3, 2013 at 12:55 AM, Mantas Mikulėnas  wrote:
>
> On Dec 3, 2013 1:51 AM, "Tom Gundersen"  wrote:
>>
>> On Tue, Dec 3, 2013 at 12:04 AM, Kay Sievers  wrote:
>> > On Mon, Dec 2, 2013 at 11:52 PM, Goffredo Baroncelli
>> >  wrote:
>> >
>> >> I have ne question: what happens if a sysctl setting is in more than
>> >> one file ? systemd-sysctl is smart enough to write the last value or
>> >>  perform several writes ?
>> >
>> > One write only, it logs at "info" level about overwritten values.
>> >
>> >>> Kay explained in IRC that we do not allow such actions, because access
>> >>> to
>> >>> the keyboad doesn't mean full access to the machine, and we default to
>> >>> safe
>> >>> settings. Allowing the reboot though logind is different, because the
>> >>> user
>> >>> must authenticate first to open a session.
>> >>
>> >> Sorry, but I cannot agree: from a theoretical point of view Kay has
>> >> reason. However who has access to the keyboard and not to the "power
>> >> switch" ? If I want to switch the PC and the software cannot allow it,
>> >> I
>> >> unplug the main power...
>> >
>> > The keyboard is surely not the computer itself, the wires or the reset
>> > or power button. Login prompts must not have the ability to trigger
>> > unsafe options with the keyboard alone.
>>
>> It is useful to imagine an internet cafe, a library, or a school,
>> where the user may only have physical access to the keyboard, and not
>> the machine itself.
>
> But logind needs to be reconfigured anyway to disallow reboots in this
> situation, so why would sysctl be different?

No, logind requires an active session of a locally logged-in user.
That is safe enough for a default.

A login prompt only should not be able to do that.

> Also Ctrl-Alt-Del and/or the login manager's Reboot option.

This will go away with when we move to systemd-consoled from kernel
VTs, it can do the same logic as logind.

Kay
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] Split sysctl 50-default.conf setting file

2013-12-02 Thread Mantas Mikulėnas
On Dec 3, 2013 1:51 AM, "Tom Gundersen"  wrote:
>
> On Tue, Dec 3, 2013 at 12:04 AM, Kay Sievers  wrote:
> > On Mon, Dec 2, 2013 at 11:52 PM, Goffredo Baroncelli 
wrote:
> >
> >> I have ne question: what happens if a sysctl setting is in more than
> >> one file ? systemd-sysctl is smart enough to write the last value or
> >>  perform several writes ?
> >
> > One write only, it logs at "info" level about overwritten values.
> >
> >>> Kay explained in IRC that we do not allow such actions, because
access to
> >>> the keyboad doesn't mean full access to the machine, and we default
to safe
> >>> settings. Allowing the reboot though logind is different, because the
user
> >>> must authenticate first to open a session.
> >>
> >> Sorry, but I cannot agree: from a theoretical point of view Kay has
> >> reason. However who has access to the keyboard and not to the "power
> >> switch" ? If I want to switch the PC and the software cannot allow it,
I
> >> unplug the main power...
> >
> > The keyboard is surely not the computer itself, the wires or the reset
> > or power button. Login prompts must not have the ability to trigger
> > unsafe options with the keyboard alone.
>
> It is useful to imagine an internet cafe, a library, or a school,
> where the user may only have physical access to the keyboard, and not
> the machine itself.

But logind needs to be reconfigured anyway to disallow reboots in this
situation, so why would sysctl be different?

Also Ctrl-Alt-Del and/or the login manager's Reboot option.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] Split sysctl 50-default.conf setting file

2013-12-02 Thread Tom Gundersen
On Tue, Dec 3, 2013 at 12:04 AM, Kay Sievers  wrote:
> On Mon, Dec 2, 2013 at 11:52 PM, Goffredo Baroncelli  
> wrote:
>
>> I have ne question: what happens if a sysctl setting is in more than
>> one file ? systemd-sysctl is smart enough to write the last value or
>>  perform several writes ?
>
> One write only, it logs at "info" level about overwritten values.
>
>>> Kay explained in IRC that we do not allow such actions, because access to
>>> the keyboad doesn't mean full access to the machine, and we default to safe
>>> settings. Allowing the reboot though logind is different, because the user
>>> must authenticate first to open a session.
>>
>> Sorry, but I cannot agree: from a theoretical point of view Kay has
>> reason. However who has access to the keyboard and not to the "power
>> switch" ? If I want to switch the PC and the software cannot allow it, I
>> unplug the main power...
>
> The keyboard is surely not the computer itself, the wires or the reset
> or power button. Login prompts must not have the ability to trigger
> unsafe options with the keyboard alone.

It is useful to imagine an internet cafe, a library, or a school,
where the user may only have physical access to the keyboard, and not
the machine itself.

>> I think that we should give access to other keys like:
>> - Boot
>> - Reboot
>> - powerOff
>> - Umount
>
> Sure it's useful for you as it is for me on my box, but it is not a
> safe default. You need to set it locally, we cannot do that.
>
> Kay
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] Split sysctl 50-default.conf setting file

2013-12-02 Thread Kay Sievers
On Mon, Dec 2, 2013 at 11:52 PM, Goffredo Baroncelli  wrote:

> I have ne question: what happens if a sysctl setting is in more than
> one file ? systemd-sysctl is smart enough to write the last value or
>  perform several writes ?

One write only, it logs at "info" level about overwritten values.

>> Kay explained in IRC that we do not allow such actions, because access to
>> the keyboad doesn't mean full access to the machine, and we default to safe
>> settings. Allowing the reboot though logind is different, because the user
>> must authenticate first to open a session.
>
> Sorry, but I cannot agree: from a theoretical point of view Kay has
> reason. However who has access to the keyboard and not to the "power
> switch" ? If I want to switch the PC and the software cannot allow it, I
> unplug the main power...

The keyboard is surely not the computer itself, the wires or the reset
or power button. Login prompts must not have the ability to trigger
unsafe options with the keyboard alone.

> I think that we should give access to other keys like:
> - Boot
> - Reboot
> - powerOff
> - Umount

Sure it's useful for you as it is for me on my box, but it is not a
safe default. You need to set it locally, we cannot do that.

Kay
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] Split sysctl 50-default.conf setting file

2013-12-02 Thread Goffredo Baroncelli
Hi Zbyszek
On 2013-12-02 23:27, Zbigniew Jędrzejewski-Szmek wrote:
> On Mon, Dec 02, 2013 at 10:27:45PM +0100, Goffredo Baroncelli wrote:
[...]
>>
>> Yes am doing so. But IIRC the process order of the sysctl file was
>> inverted near systemd 207...
>>
>> Because Debian uses 204, when it switches to something more recent than
>> 207 this setup will not work any more :-( so I have to change the order
>> number.
> Yes, that unfortunate :), but easy to work around: just install the file
> with a high number, and symlink with a low number. The symlink can be removed
> after update to 208.

Thanks, good suggestions
> 
>> Anyway I think that it is more clean to separate the setting in more files.
> This would make the number of files equal to the number of settings we are
> changing, which would be messy.

This is not the first case that a config file is split in several
sub-files. The .d directories are a typical example.

I have ne question: what happens if a sysctl setting is in more than
one file ? systemd-sysctl is smart enough to write the last value or
 perform several writes ?


>>> BTW, Kay, why is the default so conservative here (sysrq only)?
>>> I would think that the general principle that the user who has physical
>>> access to the machine and can flip the power switch should be able to
>>> do various things which are disruptive, but not are not proviledge
>>> escalation (let's call them reboot-like).
>>
>> I agree with you
> Kay explained in IRC that we do not allow such actions, because access to
> the keyboad doesn't mean full access to the machine, and we default to safe
> settings. Allowing the reboot though logind is different, because the user
> must authenticate first to open a session.

Sorry, but I cannot agree: from a theoretical point of view Kay has
reason. However who has access to the keyboard and not to the "power
switch" ? If I want to switch the PC and the software cannot allow it, I
unplug the main power...

I think that we should give access to other keys like:
- Boot
- Reboot
- powerOff
- Umount

- often my Xorg freez and syrq-K is also useful

Goffredo

> Zbyszek
> 


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] Not authorized to perform operation

2013-12-02 Thread Michael Biebl
Also install policykit-1 from experimental, which is built with logind support.

2013/12/2 Floris :
> Dear systemd maintainers,
>
> recently I upgraded Debian Testing to Gnome 3.8, but unfortunately gdm3.8
> is build without systemd support, so I installed gdm3.10 from experimental
> with his dependencies. (So basically I run Gnome 3.10)
>
> gnome-shell --version
> GNOME Shell 3.10.1
>
> When I try to mount a drive in Nautilus I get: "Unable to access "video"
> Not authorized to perform operation" There is no
> pop-up which ask for a password.
>
> Can someone point me how to solve this problem?
>
> Some information about my system:
> Please ask if you need more
>
> floris@Alice:~$ groups
> floris root sudo audio video fuse kids
>
> floris@Alice:~$ ck-list-sessions
> {empty}
>
> floris@Alice:~$ loginctl show-seat seat0
> Id=seat0
> ActiveSession=7
> CanMultiSession=yes
> CanTTY=yes
> CanGraphical=no
> Sessions=7
> IdleHint=no
> IdleSinceHint=0
> IdleSinceHintMonotonic=0
>
> floris@Alice:~$ loginctl session-status 7
> 7 - floris (1000)
>  Since: ma 2013-12-02 14:54:00 CET; 51min ago
> Leader: 5674 (gdm-session-wor)
>   Seat: seat0; vc7
>Display: :0
>Service: gdm3; type x11; class user
>  State: active
> CGroup: systemd:/user/1000.user/7.session
> ├─5674 gdm-session-worker [pam/gdm3]
> ├─5684 /usr/bin/gnome-keyring-daemon --daemonize --login
> ├─5686 x-session-manager
> ├─5727 /usr/bin/ssh-agent /usr/bin/dbus-launch
> --exit-with-...
> ├─5730 /usr/bin/dbus-launch --exit-with-session
> x-session-m...
> ├─5731 /usr/bin/dbus-daemon --fork --print-pid 5
> --print-ad...
> ├─5734 /usr/lib/at-spi2-core/at-spi-bus-launcher
> ├─5738 /usr/bin/dbus-daemon
> --config-file=/etc/at-spi2/acce...
> ├─5741 /usr/lib/at-spi2-core/at-spi2-registryd
> --use-gnome-...
> ├─5745 /usr/lib/gvfs/gvfsd
> ├─5755
> /usr/lib/gnome-settings-daemon/gnome-settings-daemon...
> ├─5769 /usr/bin/pulseaudio --start
> ├─5777 /usr/lib/gvfs/gvfs-udisks2-volume-monitor
> ├─5785 /usr/lib/gvfs/gvfs-afc-volume-monitor
> ├─5790 /usr/lib/gvfs/gvfs-gphoto2-volume-monitor
> ├─5794 /usr/lib/gvfs/gvfs-mtp-volume-monitor
> ├─5798 /usr/lib/gvfs/gvfs-goa-volume-monitor
> ├─5801 /usr/lib/gnome-online-accounts/goa-daemon
> ├─5807 /usr/lib/telepathy/mission-control-5
> ├─5810 /usr/bin/gnome-shell
> ├─5820 /usr/lib/gnome-settings-daemon/gsd-printer
> ├─5822 /usr/lib/dconf/dconf-service
> ├─5848 /usr/lib/gnome-shell/gnome-shell-calendar-server
> ├─5858 /usr/bin/python
> /usr/share/system-config-printer/app...
> ├─5862 /usr/lib/tracker/tracker-miner-fs
> ├─5865 /usr/lib/tracker/tracker-store
> ├─5866 /usr/bin/rygel
> ├─5896 /usr/lib/gnome-terminal/gnome-terminal-server
> ├─5899 gnome-pty-helper
> ├─5900 bash
> ...
>
>
> --
> To UNSUBSCRIBE, email to debian-user-requ...@lists.debian.org
> with a subject of "unsubscribe". Trouble? Contact
> listmas...@lists.debian.org
> Archive: http://lists.debian.org/op.w7gzi7le5k9...@alice.jkfloris.demon.nl
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel



-- 
Why is it that all of the instruments seeking intelligent life in the
universe are pointed away from Earth?
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] [PATCH] core: fix Unit.SetProperties argument parsing

2013-12-02 Thread David Herrmann
SetProperties has signature "ba(sv)", but the bus_unit_set_properties()
helper already does a enter_container('a', "sv") so we have to skip it in
bus_unit_method_set_properties().
---
Heyho

I just stumbled over this. Don't have any test-case and it's too late here to be
 100% sure about bus argument parsing.. Maybe someone can review that?

Thanks
David

 src/core/dbus-unit.c | 8 
 1 file changed, 8 deletions(-)

diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c
index 1fec0e3..e95a529 100644
--- a/src/core/dbus-unit.c
+++ b/src/core/dbus-unit.c
@@ -480,18 +480,10 @@ int bus_unit_method_set_properties(sd_bus *bus, 
sd_bus_message *message, void *u
 if (r < 0)
 return r;
 
-r = sd_bus_message_enter_container(message, 'a', "(sv)");
-if (r < 0)
-return r;
-
 r = bus_unit_set_properties(u, message, runtime ? UNIT_RUNTIME : 
UNIT_PERSISTENT, true, error);
 if (r < 0)
 return r;
 
-r = sd_bus_message_exit_container(message);
-if (r < 0)
-return r;
-
 return sd_bus_reply_method_return(message, NULL);
 }
 
-- 
1.8.4.2

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] Split sysctl 50-default.conf setting file

2013-12-02 Thread Zbigniew Jędrzejewski-Szmek
On Mon, Dec 02, 2013 at 10:27:45PM +0100, Goffredo Baroncelli wrote:
> On 2013-12-02 21:38, Zbigniew Jędrzejewski-Szmek wrote:
> > On Mon, Dec 02, 2013 at 09:15:37PM +0100, Goffredo Baroncelli wrote:
> >> Hi all,
> >>
> >> currently systemd contains a sysctl default setting in a file called
> >> 50-default.conf
> >> The aim of this patch is to split the content of the sysctl setting in
> >> more files to allow a more selective override.
> > Hi Goffredo,
> > I think that the misunderstading is that you *can* override invidual
> > settings. If you provide a file with a name higher in order, containing
> > just sysctl.sysrq override, just this setting will be overriden.
> 
> Yes am doing so. But IIRC the process order of the sysctl file was
> inverted near systemd 207...
> 
> Because Debian uses 204, when it switches to something more recent than
> 207 this setup will not work any more :-( so I have to change the order
> number.
Yes, that unfortunate :), but easy to work around: just install the file
with a high number, and symlink with a low number. The symlink can be removed
after update to 208.

> Anyway I think that it is more clean to separate the setting in more files.
This would make the number of files equal to the number of settings we are
changing, which would be messy.

> > BTW, Kay, why is the default so conservative here (sysrq only)?
> > I would think that the general principle that the user who has physical
> > access to the machine and can flip the power switch should be able to
> > do various things which are disruptive, but not are not proviledge
> > escalation (let's call them reboot-like).
> 
> I agree with you
Kay explained in IRC that we do not allow such actions, because access to
the keyboad doesn't mean full access to the machine, and we default to safe
settings. Allowing the reboot though logind is different, because the user
must authenticate first to open a session.

Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] Split sysctl 50-default.conf setting file

2013-12-02 Thread Goffredo Baroncelli
On 2013-12-02 21:32, Kay Sievers wrote:
> On Mon, Dec 2, 2013 at 9:15 PM, Goffredo Baroncelli  
> wrote:
>> currently systemd contains a sysctl default setting in a file called
>> 50-default.conf
>> The aim of this patch is to split the content of the sysctl setting in
>> more files to allow a more selective override.
>>
>> My need is to enable all the sysrq key. Instead systemd defaults is to
>> disallow all sysrq keys except the sync one [1].
>> To do that, I would have to override the sysctl file
>> /usr/lib/sysctl.d/50-default.conf file,
>> putting a file with the same name in
>> /etc/sysctl.d
>> However this file contains other settings than the one which I want to
>> override; so I would lost any update of these other settings made by
>> upstream.
>>
>> With this patch I am able to override only the setting related to the sysrq.
> 
> You should be able to overwrite individual settings just fine. I don't
> think this is needed.

What happens if the same sysctl is present is in two files: the value is
written two times, or systemd-sysctl is smart enough to write only the
last one ?

I have to point out that I spent some time to find who changed this
setting when I installed systemd. A more explicit name file would helped.

> 
>> create mode 100644 sysctl.d/50-default_kernel_sysrq.conf
> 
> We usually don't do "_" in file names. :)

Just for curiosity: there is a rationale or it is a convention (I am
fine with removing "_", but I am curious about the reason)
> 
> Kay
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
> 


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] Not authorized to perform operation

2013-12-02 Thread Floris

Dear systemd maintainers,

recently I upgraded Debian Testing to Gnome 3.8, but unfortunately gdm3.8
is build without systemd support, so I installed gdm3.10 from experimental
with his dependencies. (So basically I run Gnome 3.10)

gnome-shell --version
GNOME Shell 3.10.1

When I try to mount a drive in Nautilus I get: "Unable to access "video"
Not authorized to perform operation" There is no
pop-up which ask for a password.

Can someone point me how to solve this problem?

Some information about my system:
Please ask if you need more

floris@Alice:~$ groups
floris root sudo audio video fuse kids

floris@Alice:~$ ck-list-sessions
{empty}

floris@Alice:~$ loginctl show-seat seat0
Id=seat0
ActiveSession=7
CanMultiSession=yes
CanTTY=yes
CanGraphical=no
Sessions=7
IdleHint=no
IdleSinceHint=0
IdleSinceHintMonotonic=0

floris@Alice:~$ loginctl session-status 7
7 - floris (1000)
 Since: ma 2013-12-02 14:54:00 CET; 51min ago
Leader: 5674 (gdm-session-wor)
  Seat: seat0; vc7
   Display: :0
   Service: gdm3; type x11; class user
 State: active
CGroup: systemd:/user/1000.user/7.session
├─5674 gdm-session-worker [pam/gdm3]
├─5684 /usr/bin/gnome-keyring-daemon --daemonize  
--login

├─5686 x-session-manager
├─5727 /usr/bin/ssh-agent /usr/bin/dbus-launch
--exit-with-...
├─5730 /usr/bin/dbus-launch --exit-with-session
x-session-m...
├─5731 /usr/bin/dbus-daemon --fork --print-pid 5
--print-ad...
├─5734 /usr/lib/at-spi2-core/at-spi-bus-launcher
├─5738 /usr/bin/dbus-daemon
--config-file=/etc/at-spi2/acce...
├─5741 /usr/lib/at-spi2-core/at-spi2-registryd
--use-gnome-...
├─5745 /usr/lib/gvfs/gvfsd
├─5755
/usr/lib/gnome-settings-daemon/gnome-settings-daemon...
├─5769 /usr/bin/pulseaudio --start
├─5777 /usr/lib/gvfs/gvfs-udisks2-volume-monitor
├─5785 /usr/lib/gvfs/gvfs-afc-volume-monitor
├─5790 /usr/lib/gvfs/gvfs-gphoto2-volume-monitor
├─5794 /usr/lib/gvfs/gvfs-mtp-volume-monitor
├─5798 /usr/lib/gvfs/gvfs-goa-volume-monitor
├─5801 /usr/lib/gnome-online-accounts/goa-daemon
├─5807 /usr/lib/telepathy/mission-control-5
├─5810 /usr/bin/gnome-shell
├─5820 /usr/lib/gnome-settings-daemon/gsd-printer
├─5822 /usr/lib/dconf/dconf-service
├─5848 /usr/lib/gnome-shell/gnome-shell-calendar-server
├─5858 /usr/bin/python
/usr/share/system-config-printer/app...
├─5862 /usr/lib/tracker/tracker-miner-fs
├─5865 /usr/lib/tracker/tracker-store
├─5866 /usr/bin/rygel
├─5896 /usr/lib/gnome-terminal/gnome-terminal-server
├─5899 gnome-pty-helper
├─5900 bash
...


--
To UNSUBSCRIBE, email to debian-user-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact
listmas...@lists.debian.org
Archive: http://lists.debian.org/op.w7gzi7le5k9...@alice.jkfloris.demon.nl
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] [HACK/RFC/PATCH] systemd-su: "su" on steroids

2013-12-02 Thread David Herrmann
So people continue asking how to spawn a process in a new session from the
command-line. Turns out, it's not as easy as you might think. I stopped at
some point trying, but something like this should work:
  systemd-run --description="Test" -- /bin/sh -- XDG_SEAT=seat0 XDG_VTNR=5 su - 
david -- /some/command

However, without modifying the pam-su configuration, this still doesn't
spawn a new session (su takes over the existing session). "sudo" allows
changing the pam-service to "login" on the command line, so maybe that
would work..

Fortunately, I was hacking on systemd-welcomed the last few days and had
to deal with PAM, anyway. So I thought why no hack on a small "su" helper
which spawns the given command in a fresh new session. As a bonus, I
wanted to pass file-descriptors to the new process, so I could run bash in
a new session but connected to STDIN/OUT of the caller.

4h later, I present "systemd-su":

If you want to give it a try, run:
  systemd-su -u david /bin/sh

It requires the systemd-suexec helper internally, so if you didn't install
it, use something like this:
  SYSTEMD_SUEXEC=/home/david/dev/systemd/systemd-suexec ./systemd-su -u david sh
Otherwise, systemd-su cannot find the systemd-suexec binary.

Last but not least, systemd-su is probably not SETUID, so run it via sudo!

What does this do?
Quite easy: systemd-su parses it's arguments, rearranges them and prepends
them by "systemd-suexec  -- ". Then it
passes this to systemd and spawns it as transient unit (like systemd-run).
The systemd-suexec helper uses pam to do the initialization dance, creates
the new session and then spawns your command in it.

Additionally, I create an anonymous AF_UNIX socket in systemd-su which
systemd-suexec connects to. I then pass file-descriptors to systemd-suexec
which installes them as STDIN/OUT/ERR.
I actually think it would be quite useful to extend the
StartTransientUnit() dbus call to allow passing FDs. It would allow us to
"store" FDs with active units, which would be useful for a lot of other
concepts (like storing DRM framebuffer handles..).

This also requires /etc/pam.d/systemd-suexec to be:
#%PAM-1.0
# Used by systemd-suexec.
authrequisite   pam_nologin.so
authsufficient  pam_rootok.so
authrequiredpam_deny.so
account requiredpam_nologin.so
account requiredpam_unix.so
passwordrequiredpam_deny.so
session requiredpam_unix.so
session requiredpam_systemd.so
session requiredpam_env.so
Or something equivalent..

Also note, this is a HACK! I can run /bin/sh now, but this helper still
lacks several features and I'm not sure it's that useful to other people..
I just thought sharing it doesn't hurt.

Cheers
David
---
 .gitignore   |   2 +
 Makefile.am  |  38 +
 src/shared/session-pam.c | 394 +++
 src/shared/session-pam.h |  53 ++
 src/su/Makefile  |   1 +
 src/su/su.c  | 429 +++
 src/su/suexec.c  | 377 +
 7 files changed, 1294 insertions(+)
 create mode 100644 src/shared/session-pam.c
 create mode 100644 src/shared/session-pam.h
 create mode 12 src/su/Makefile
 create mode 100644 src/su/su.c
 create mode 100644 src/su/suexec.c

diff --git a/.gitignore b/.gitignore
index 9fd42d5..22298b3 100644
--- a/.gitignore
+++ b/.gitignore
@@ -78,6 +78,8 @@
 /systemd-shutdownd
 /systemd-sleep
 /systemd-bus-proxyd
+/systemd-su
+/systemd-suexec
 /systemd-sysctl
 /systemd-system-update-generator
 /systemd-timedated
diff --git a/Makefile.am b/Makefile.am
index 5c65281..b5788a9 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1880,6 +1880,44 @@ systemd_run_LDADD = \
libsystemd-shared.la
 
 # 
--
+if ENABLE_LOGIND
+bin_PROGRAMS += \
+   systemd-su
+rootlibexec_PROGRAMS += \
+   systemd-suexec
+
+systemd_su_SOURCES = \
+   src/su/su.c
+
+systemd_su_CFLAGS = \
+   $(AM_CFLAGS)
+
+systemd_su_LDADD = \
+   libsystemd-label.la \
+   libsystemd-capability.la \
+   libsystemd-bus-internal.la \
+   libsystemd-login-internal.la \
+   libsystemd-daemon-internal.la \
+   libsystemd-id128-internal.la \
+   libsystemd-shared.la
+
+systemd_suexec_SOURCES = \
+   src/su/suexec.c \
+   src/shared/session-pam.h \
+   src/shared/session-pam.c
+
+systemd_suexec_CFLAGS = \
+   $(AM_CFLAGS) \
+   $(PAM_CLFAGS)
+
+systemd_suexec_LDADD = \
+   $(PAM_LIBS) \
+   libsystemd-login-internal.la \
+   libsystemd-daemon-internal.la \
+   libsystemd-shared.la
+endif
+
+# 
--
 systemd_bus_proxyd_SOURCES = \
src/bus-proxyd/bus-proxyd.c
 
diff 

Re: [systemd-devel] [PATCH] Split sysctl 50-default.conf setting file

2013-12-02 Thread Zbigniew Jędrzejewski-Szmek
On Mon, Dec 02, 2013 at 09:15:37PM +0100, Goffredo Baroncelli wrote:
> Hi all,
> 
> currently systemd contains a sysctl default setting in a file called
> 50-default.conf
> The aim of this patch is to split the content of the sysctl setting in
> more files to allow a more selective override.
Hi Goffredo,
I think that the misunderstading is that you *can* override invidual
settings. If you provide a file with a name higher in order, containing
just sysctl.sysrq override, just this setting will be overriden.

BTW, Kay, why is the default so conservative here (sysrq only)?
I would think that the general principle that the user who has physical
access to the machine and can flip the power switch should be able to
do various things which are disruptive, but not are not proviledge
escalation (let's call them reboot-like).

> +#   1 - enable all functions of sysrq
> +#  >1 - bitmask of allowed sysrq functions (see below for detailed function
> +#   description):
> +#  2 - enable control of console logging level
> +#  4 - enable control of keyboard (SAK, unraw)
> +#  8 - enable debugging dumps of processes etc.
> +# 16 - enable sync command
> +# 32 - enable remount read-only
> +# 64 - enable signalling of processes (term, kill, oom-kill)
> +#128 - allow reboot/poweroff
> +#256 - allow nicing of all RT tasks
> +
> +kernel.sysrq = 16 # only enable sync command

Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] Split sysctl 50-default.conf setting file

2013-12-02 Thread Kay Sievers
On Mon, Dec 2, 2013 at 9:15 PM, Goffredo Baroncelli  wrote:
> currently systemd contains a sysctl default setting in a file called
> 50-default.conf
> The aim of this patch is to split the content of the sysctl setting in
> more files to allow a more selective override.
>
> My need is to enable all the sysrq key. Instead systemd defaults is to
> disallow all sysrq keys except the sync one [1].
> To do that, I would have to override the sysctl file
> /usr/lib/sysctl.d/50-default.conf file,
> putting a file with the same name in
> /etc/sysctl.d
> However this file contains other settings than the one which I want to
> override; so I would lost any update of these other settings made by
> upstream.
>
> With this patch I am able to override only the setting related to the sysrq.

You should be able to overwrite individual settings just fine. I don't
think this is needed.

> create mode 100644 sysctl.d/50-default_kernel_sysrq.conf

We usually don't do "_" in file names. :)

Kay
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] [PATCH] Split sysctl 50-default.conf setting file

2013-12-02 Thread Goffredo Baroncelli
Hi all,

currently systemd contains a sysctl default setting in a file called
50-default.conf
The aim of this patch is to split the content of the sysctl setting in
more files to allow a more selective override.

My need is to enable all the sysrq key. Instead systemd defaults is to
disallow all sysrq keys except the sync one [1].
To do that, I would have to override the sysctl file
/usr/lib/sysctl.d/50-default.conf file,
putting a file with the same name in
/etc/sysctl.d
However this file contains other settings than the one which I want to
override; so I would lost any update of these other settings made by
upstream.

With this patch I am able to override only the setting related to the sysrq.

Please apply.

BR
G.Baroncelli


[1] For the record, I am against this kind of setting. I opened a bug
in debian (#725422), but it was suggested me to send a patch to upstream.
Of course it is in the systemd right to set whatever default it thinks sane.

Signed-off-by: Goffredo Baroncelli 
---
 Makefile.am   |  4 +++-
 sysctl.d/50-coredump.conf.in  |  3 +++
 sysctl.d/50-default.conf  | 24 
 sysctl.d/50-default_fs.conf   | 12 
 sysctl.d/50-default_kernel_sysrq.conf | 26 ++
 sysctl.d/50-default_net.conf  | 14 ++
 6 files changed, 58 insertions(+), 25 deletions(-)
 delete mode 100644 sysctl.d/50-default.conf
 create mode 100644 sysctl.d/50-default_fs.conf
 create mode 100644 sysctl.d/50-default_kernel_sysrq.conf
 create mode 100644 sysctl.d/50-default_net.conf

diff --git a/Makefile.am b/Makefile.am
index 90874df..95087c6 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -357,7 +357,9 @@ dist_zshcompletion_DATA = \
shell-completion/zsh/_systemd
  dist_sysctl_DATA = \
-   sysctl.d/50-default.conf
+   sysctl.d/50-default_kernel_sysrq.conf \
+   sysctl.d/50-default_net.conf \
+   sysctl.d/50-default_fs.conf
  dist_systemunit_DATA = \
units/graphical.target \
diff --git a/sysctl.d/50-coredump.conf.in b/sysctl.d/50-coredump.conf.in
index d5795a3..1db1047 100644
--- a/sysctl.d/50-coredump.conf.in
+++ b/sysctl.d/50-coredump.conf.in
@@ -8,3 +8,6 @@
 # See sysctl.d(5) and core(5) for for details.
  kernel.core_pattern=|@rootlibexecdir@/systemd-coredump %p %u %g %s %t %e
+
+# Append the PID to the core filename
+kernel.core_uses_pid = 1
diff --git a/sysctl.d/50-default.conf b/sysctl.d/50-default.conf
deleted file mode 100644
index 46bae21..000
--- a/sysctl.d/50-default.conf
+++ /dev/null
@@ -1,24 +0,0 @@
-#  This file is part of systemd.
-#
-#  systemd is free software; you can redistribute it and/or modify it
-#  under the terms of the GNU Lesser General Public License as published by
-#  the Free Software Foundation; either version 2.1 of the License, or
-#  (at your option) any later version.
-
-# See sysctl.d(5) and core(5) for for details.
-
-# System Request functionality of the kernel (SYNC)
-kernel.sysrq = 16
-
-# Append the PID to the core filename
-kernel.core_uses_pid = 1
-
-# Source route verification
-net.ipv4.conf.default.rp_filter = 1
-
-# Do not accept source routing
-net.ipv4.conf.default.accept_source_route = 0
-
-# Enable hard and soft link protection
-fs.protected_hardlinks = 1
-fs.protected_symlinks = 1
diff --git a/sysctl.d/50-default_fs.conf b/sysctl.d/50-default_fs.conf
new file mode 100644
index 000..a2e7eb4
--- /dev/null
+++ b/sysctl.d/50-default_fs.conf
@@ -0,0 +1,12 @@
+#  This file is part of systemd.
+#
+#  systemd is free software; you can redistribute it and/or modify it
+#  under the terms of the GNU Lesser General Public License as published by
+#  the Free Software Foundation; either version 2.1 of the License, or
+#  (at your option) any later version.
+
+# See sysctl.d(5) for for details.
+
+# Enable hard and soft link protection
+fs.protected_hardlinks = 1
+fs.protected_symlinks = 1
diff --git a/sysctl.d/50-default_kernel_sysrq.conf
b/sysctl.d/50-default_kernel_sysrq.conf
new file mode 100644
index 000..a848745
--- /dev/null
+++ b/sysctl.d/50-default_kernel_sysrq.conf
@@ -0,0 +1,26 @@
+#  This file is part of systemd.
+#
+#  systemd is free software; you can redistribute it and/or modify it
+#  under the terms of the GNU Lesser General Public License as published by
+#  the Free Software Foundation; either version 2.1 of the License, or
+#  (at your option) any later version.
+
+# See sysctl.d(5) for for details.
+
+# From Documentation/sysrq.txt: possible value to control which sysrq
+# could be invoked from keyboard
+#
+#   0 - disable sysrq completely
+#   1 - enable all functions of sysrq
+#  >1 - bitmask of allowed sysrq functions (see below for detailed function
+#   description):
+#  2 - enable control of console logging level
+#  4 - enable control of keyboard (SAK, unraw)
+#  8 - enable debugging dumps of processes etc.
+# 16 - enable sync command
+#   

Re: [systemd-devel] [PATCH 2/7] Give the user permissions to their session's cgroup

2013-12-02 Thread Hristo Venev
See systemd src/core/execute.c:1299-1312, especially lines 1300 and
1307.

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [systemd-commits] 4 commits - catalog/systemd-fr.catalog catalog/systemd-ru.catalog configure.ac Makefile.am po/.gitignore po/LINGUAS po/ru.po

2013-12-02 Thread Lennart Poettering
On Mon, 02.12.13 19:25, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote:

> > I must say I really dislike mixing declarations and code. And I also
> > know that by accident I did this in many cases myself and was thankful
> > for the compiler to tell me about this.
>
> I never understood this. There are many situations where initializing
> variables right when they are declared leads to more readable code.
> And if everything has to be declared at the top, this often isn't
> possible. Especially with long functions which have many logically
> separate segments this leads to an unreadable lump of declarations at
> the top. We already declare variables at the start of internal scopes,
> and declaring them in the middle of code is just another small step.

I figure this means that your function is probably too large and you
should actually just split it up into many ... That's almost definitely
the best option usually...

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] The whole su/pkexec session debate

2013-12-02 Thread Colin Walters
On Mon, 2013-12-02 at 14:37 +0100, David Herrmann wrote:

> But then gnome-session should simply call ReleaseSession() on the bus itself..

I'd rather have some sort of API where a particular process is the
"session leader", and its exit implies closing.  Something like a pid
file in /run/systemd/sessions/c0/leader/pid which is owned by the
session's uid/gid?



___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [systemd-commits] 4 commits - catalog/systemd-fr.catalog catalog/systemd-ru.catalog configure.ac Makefile.am po/.gitignore po/LINGUAS po/ru.po

2013-12-02 Thread Zbigniew Jędrzejewski-Szmek
On Mon, Dec 02, 2013 at 04:58:40PM +0100, Lennart Poettering wrote:
> On Sun, 01.12.13 01:26, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote:
> 
> > 
> > On Fri, Nov 29, 2013 at 02:45:21PM -0500, Colin Walters wrote:
> > > On Fri, 2013-11-29 at 02:39 +0100, Lennart Poettering wrote:
> > > > On Thu, 28.11.13 00:54, Zbigniew Jędrzejewski-Szmek 
> > > > (zbys...@kemper.freedesktop.org) wrote:
> > > 
> > > > Instead we should add some code to macro.h which turns off the the
> > > > warning with the #pragma stuff only if it detects it is being run on an
> > > > old gcc.
> > > 
> > > There's also this approach:
> > > 
> > > 
> > 
> > > From 7affb075dd1889fbb6b8d8865dec4b5e1d36448f Mon Sep 17 00:00:00 2001
> > > From: Colin Walters 
> > > Date: Fri, 29 Nov 2013 14:43:45 -0500
> > > Subject: [PATCH] macro: Split assert_cc, add assert_cc_toplevel
> > > 
> > > To suppress warnings about -Wdeclaration-after-statement, we need to
> > > wrap static asserts inside functions with a standard do {} while(0)
> > > block.  But the toplevel asserts can't have that, so add
> > > assert_cc_toplevel for those.
> > Actually I don't think we need to totally forbid declarations after 
> > statements.
> > This is not something likely to be introduced by mistake, so I think
> > we'll be fine even if only humans check for that.
> 
> I must say I really dislike mixing declarations and code. And I also
> know that by accident I did this in many cases myself and was thankful
> for the compiler to tell me about this.
I never understood this. There are many situations where initializing
variables right when they are declared leads to more readable
code. And if everything has to be declared at the top, this often
isn't possible. Especially with long functions which have many
logically separate segments this leads to an unreadable lump of
declarations at the top. We already declare variables at the start of
internal scopes, and declaring them in the middle of code is just
another small step.

Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH 1/2] nspawn: --populate to run static binaries on empty target directory

2013-12-02 Thread Shawn Landden
On Mon, Dec 2, 2013 at 8:27 AM, Lennart Poettering
 wrote:
> On Sat, 30.11.13 10:20, Shawn Landden (sh...@churchofgit.com) wrote:
>
>> nspawn has been called "chroot on steroids".
>>
>> Continue that tradition by supporting target directories that
>> are not root directories.
>>
>> This patch handles the simple case: a static binary.
>
> Hmm, I am not sure how I feel about this. This appears a bit too
> specific for me, and given the requirement for static binaries this is
> also so limited.
The next patch is the series adds support for dynamic libraries. This patch
also doesn't need bind mounts, and it executes through /proc/self/fd/%n, but
support for one-file scripts and dynamic libraries in the next patch does
require bind mounts. I feel you don't really understand my patch. :/
I'll sum up what I'm doing:

If --populate is passed, analyze the executable, which opens it and set the
exec path to /proc/self/fd/%n. If executable is static this is all you
have to do.

If it has a shebang, analyze that. If either the shebang or executable
is dynamic,
test if the linker is the GNU linker, and if it is have the linker
tell use what libraries the
executable needs. Then bind mount the linker, shebang (if there is one), and
libraries into the target.

Unmount these when the machine shuts down.
>
> I wonder if we can find a different way to support this, without adding
> high-level switches to nspawn itself.
>
> For example, couldn't extending "--bind=" a bit to also support bind
> mounting files (in contrast to just directories the way it currently
> does) already gets us 90% of the way? And then do the rest 10% by adding
> an example how to use this to bind mount static binaries from the host
> into the container to the example in the man page? Allowing bind
> mounting of files has been on the TODO list for a while anyway...
>
> Something like:
>
> # systemd-nspawn -D /srv/mycontainer 
> --bind=/usr/bin/populate-container:/tmp/populate-container 
> /tmp/populate-container
>
> This of course wouldn't check if the file executed is staticall linked,
> but the user should quickly get an error about missing .sos if it isn't?
No, the linker would be missing, and the user would get "execvpe()
failed: No such file or directory",
which is confusing.
>
>>  assert_se(sigemptyset(&mask) == 0);
>> @@ -1164,7 +1195,7 @@ int main(int argc, char *argv[]) {
>>  gid_t gid = (gid_t) -1;
>>  unsigned n_env = 2;
>>  const char *envp[] = {
>> -
>> "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
>> +DEFAULT_PATH_SPLIT_USR,
>
> This bit looks like like something we really should do though. Could you
> isolate this out and resubmit, please?

>
>> +#define DEFAULT_PATH_SPLIT_USR 
>> "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
>> +
>>  #ifdef HAVE_SPLIT_USR
>> -#  define DEFAULT_PATH 
>> "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
>> +#  define DEFAULT_PATH DEFAULT_PATH_SPLIT_USR
>>  #else
>>  #  define DEFAULT_PATH "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin"
>>  #endif
>> @@ -51,6 +53,7 @@ int path_is_mount_point(const char *path, bool 
>> allow_symlink);
>>  int path_is_read_only_fs(const char *path);
>>  int path_is_os_tree(const char *path);
>
> And this too, of course...
>
> Lennart
>
> --
> Lennart Poettering, Red Hat
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [RFC] xyzctl-common

2013-12-02 Thread Lennart Poettering
On Sat, 16.11.13 19:41, Peeters Simon (peeters.si...@gmail.com) wrote:

> hello all,
> 
> During the sd_bus porting I noted that the *ctl tools (and
> systemd-analyze) contain a lot of common boilerplate code.
> 
> So the basic idea is to split this boilerplate out into
> xyzctl-common.[ch] so that f.ex the bottom hostnamectl.c would look
> like:

There actually has been a lon-standing TODO item to unify the code that
handles the "verbs" parsing in the various tools. In fact there's
currently a bit too much copy/paste going on. For example, we have
parsing code handling this in timedatectl, localectl, loginctl,
systemctl at least. They are not identical bits of code though, but very
similar (systemctl has some special magic to print bus errors only in
some cases...). But either way: this should really be unified.

> enum tool_flags {
> XYZCTL_NO_REMOTE = 1,
> Do not allow the use of --host=HOST to connect to a remote host
> XYZCTL_NO_MACHINE = 2,
> Do not allow the use of --machine=CONTAINER to connect to a container
> XYZCTL_NO_USER = 4,
> Do not allow connection to the user bus by using --user

I really don't like "negative" bools in our code. It's bad enough we
support them as command line switches, but internally we shouldn't
create more like that...

> XYZCTL_CONNECT_SYSTEMD = 8,
> connect directly to systemd's private socket.
> }

Hmmm, I do like the verb_flags thing but I am not sold to the tool_flags
idea... the systemd conenction thing for example appears so specific,
that I wouldn't want to generalize that, especially since its going to
go away with kdbus.

So, I think for now, I'd just like to see unification of
loginctl_main(), timedatectl_main(), localectl_main(),
hostnamectl_main(), ... covering the verb flags, but I'd like to avoid
the tool flags for now, let's leave this explicit in the tools, we can
generalize this later on, should it really be necessary after kdbus is
a requirement and we can drop the magic dbus code...

Anyway, all work on this appreciated!

(and srry for the long delay in reviewing, drowning in patches...)

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] Display synthetic message serial number in a more readable format than (uint32_t) -1

2013-12-02 Thread Lennart Poettering
On Mon, 02.12.13 16:31, Lukasz Skalski (l.skal...@partner.samsung.com) wrote:

> Serial=4294967295 field in message dump generated by bus_message_dump()
> function for synthetic messages isn't good readable.

Thanks! Applied!

> ---
>  src/libsystemd-bus/bus-dump.c | 12 +---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/src/libsystemd-bus/bus-dump.c b/src/libsystemd-bus/bus-dump.c
> index 469f7ba..71de081 100644
> --- a/src/libsystemd-bus/bus-dump.c
> +++ b/src/libsystemd-bus/bus-dump.c
> @@ -56,15 +56,21 @@ int bus_message_dump(sd_bus_message *m, FILE *f, bool 
> with_header) {
>  
>  if (with_header) {
>  fprintf(f,
> -"%s%s%sType=%s%s%s  Endian=%c  Flags=%u  Version=%u  
> Serial=%u ",
> +"%s%s%sType=%s%s%s  Endian=%c  Flags=%u  Version=%u",
>  m->header->type == SD_BUS_MESSAGE_METHOD_ERROR ? 
> ansi_highlight_red() :
>  m->header->type == SD_BUS_MESSAGE_METHOD_RETURN ? 
> ansi_highlight_green() :
>  m->header->type != SD_BUS_MESSAGE_SIGNAL ? 
> ansi_highlight() : "", draw_special_char(DRAW_TRIANGULAR_BULLET), 
> ansi_highlight_off(),
>  ansi_highlight(), 
> bus_message_type_to_string(m->header->type), ansi_highlight_off(),
>  m->header->endian,
>  m->header->flags,
> -m->header->version,
> -BUS_MESSAGE_SERIAL(m));
> +m->header->version);
> +
> +/* Display synthetic message serial number in a more readable
> + * format than (uint32_t) -1 */
> +if (BUS_MESSAGE_SERIAL(m) == 0xULL)
> +fprintf(f, " Serial=-1");
> +else
> +fprintf(f, " Serial=%u", BUS_MESSAGE_SERIAL(m));
>  
>  if (m->reply_serial != 0)
>  fprintf(f, "  ReplySerial=%u", m->reply_serial);


Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH 1/4] nspawn: shorten conditional path

2013-12-02 Thread Lennart Poettering
On Sun, 01.12.13 14:50, Shawn Landden (sh...@churchofgit.com) wrote:

> ---
>  src/nspawn/nspawn.c | 19 +++
>  1 file changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
> index dd7337b..0151cf3 100644
> --- a/src/nspawn/nspawn.c
> +++ b/src/nspawn/nspawn.c
> @@ -481,10 +481,8 @@ static int setup_timezone(const char *dest) {
>  return 0;
>  }
>  
> -z = path_startswith(p, "../usr/share/zoneinfo/");
> -if (!z)
> -z = path_startswith(p, "/usr/share/zoneinfo/");
> -if (!z) {
> +if ((z = path_startswith(p, "../usr/share/zoneinfo/")) ||
> +(z = path_startswith(p, "/usr/share/zoneinfo/"))) {

Please don't do this. Let's keep our code simply, let's not munge
assignments and checks into one here. We used to do that quite
extensively, but we are actually in the process of converting everything
to the simpler to read scheme where we do assignment and checks in two
steps...

(Well, ther are a few exceptions where doing this one is OK, for
example, inside the checks of while() blocks...)

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH 1/2] nspawn: --populate to run static binaries on empty target directory

2013-12-02 Thread Lennart Poettering
On Sat, 30.11.13 10:20, Shawn Landden (sh...@churchofgit.com) wrote:

> nspawn has been called "chroot on steroids".
> 
> Continue that tradition by supporting target directories that
> are not root directories.
> 
> This patch handles the simple case: a static binary.

Hmm, I am not sure how I feel about this. This appears a bit too
specific for me, and given the requirement for static binaries this is
also so limited.

I wonder if we can find a different way to support this, without adding
high-level switches to nspawn itself.

For example, couldn't extending "--bind=" a bit to also support bind
mounting files (in contrast to just directories the way it currently
does) already gets us 90% of the way? And then do the rest 10% by adding
an example how to use this to bind mount static binaries from the host
into the container to the example in the man page? Allowing bind
mounting of files has been on the TODO list for a while anyway...

Something like:

# systemd-nspawn -D /srv/mycontainer 
--bind=/usr/bin/populate-container:/tmp/populate-container 
/tmp/populate-container

This of course wouldn't check if the file executed is staticall linked,
but the user should quickly get an error about missing .sos if it isn't?

>  assert_se(sigemptyset(&mask) == 0);
> @@ -1164,7 +1195,7 @@ int main(int argc, char *argv[]) {
>  gid_t gid = (gid_t) -1;
>  unsigned n_env = 2;
>  const char *envp[] = {
> -
> "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
> +DEFAULT_PATH_SPLIT_USR,

This bit looks like like something we really should do though. Could you
isolate this out and resubmit, please?

> +#define DEFAULT_PATH_SPLIT_USR 
> "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
> +
>  #ifdef HAVE_SPLIT_USR
> -#  define DEFAULT_PATH 
> "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
> +#  define DEFAULT_PATH DEFAULT_PATH_SPLIT_USR
>  #else
>  #  define DEFAULT_PATH "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin"
>  #endif
> @@ -51,6 +53,7 @@ int path_is_mount_point(const char *path, bool 
> allow_symlink);
>  int path_is_read_only_fs(const char *path);
>  int path_is_os_tree(const char *path);

And this too, of course...

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] build broken on clang

2013-12-02 Thread Lennart Poettering
On Sun, 01.12.13 00:38, Thomas H.P. Andersen (pho...@gmail.com) wrote:

> Hi,
> 
> Since 777d7a6123cbb192a8ff9e4ac5c05b1da84b4217 the build is broken on clang:
> 
> src/libsystemd-bus/bus-control.c:686:41: error: fields must have a
> constant size: 'variable length array in structure' extension will
> never be supported
> uint8_t buffer[sz];
> ^
> src/libsystemd-bus/bus-control.c:748:33: error: fields must have a
> constant size: 'variable length array in structure' extension will
> never be supported
> uint8_t buffer[sz];

Hmm, this isn't even a struct, it's a union...

Anyway, I changed this now, given that we can use alloca0() here, and
the resulting code is actually nicer than before...

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [systemd-commits] 4 commits - catalog/systemd-fr.catalog catalog/systemd-ru.catalog configure.ac Makefile.am po/.gitignore po/LINGUAS po/ru.po

2013-12-02 Thread Lennart Poettering
On Sun, 01.12.13 10:13, Colin Walters (walt...@verbum.org) wrote:

> 
> On Sun, 2013-12-01 at 01:26 +0100, Zbigniew Jędrzejewski-Szmek wrote:
> 
> > Actually I don't think we need to totally forbid declarations after 
> > statements.
> 
> I don't have an opinion myself on making -Wdeclaration-after-statement
> an error or not, but presently with GCC 4.7 as in gnome-continuous,
> we get a spam of warnings:
> 
> http://build.gnome.org/continuous/buildmaster/builds/2013/11/30/25/build/warnings-systemd.txt
> 
> And the goal of the patch is to quiet those with older gcc versions.
> 
> (I briefly investigated the memset one but couldn't find it...)

Note that we now use "-flto" during optimized builds which will make
systemd quite a bit smaller, but will totally destroy the usefulness of
the compiler warnings on current gcc versions. You might want to turn
off lto explicitly for gnome-continuous for now...

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [systemd-commits] 4 commits - catalog/systemd-fr.catalog catalog/systemd-ru.catalog configure.ac Makefile.am po/.gitignore po/LINGUAS po/ru.po

2013-12-02 Thread Lennart Poettering
On Sun, 01.12.13 01:26, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote:

> 
> On Fri, Nov 29, 2013 at 02:45:21PM -0500, Colin Walters wrote:
> > On Fri, 2013-11-29 at 02:39 +0100, Lennart Poettering wrote:
> > > On Thu, 28.11.13 00:54, Zbigniew Jędrzejewski-Szmek 
> > > (zbys...@kemper.freedesktop.org) wrote:
> > 
> > > Instead we should add some code to macro.h which turns off the the
> > > warning with the #pragma stuff only if it detects it is being run on an
> > > old gcc.
> > 
> > There's also this approach:
> > 
> > 
> 
> > From 7affb075dd1889fbb6b8d8865dec4b5e1d36448f Mon Sep 17 00:00:00 2001
> > From: Colin Walters 
> > Date: Fri, 29 Nov 2013 14:43:45 -0500
> > Subject: [PATCH] macro: Split assert_cc, add assert_cc_toplevel
> > 
> > To suppress warnings about -Wdeclaration-after-statement, we need to
> > wrap static asserts inside functions with a standard do {} while(0)
> > block.  But the toplevel asserts can't have that, so add
> > assert_cc_toplevel for those.
> Actually I don't think we need to totally forbid declarations after 
> statements.
> This is not something likely to be introduced by mistake, so I think
> we'll be fine even if only humans check for that.

I must say I really dislike mixing declarations and code. And I also
know that by accident I did this in many cases myself and was thankful
for the compiler to tell me about this.

On newer gcc where static assert works correctly without warnings we
should totally make use of this with our current compiler flags.

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [systemd-commits] 4 commits - catalog/systemd-fr.catalog catalog/systemd-ru.catalog configure.ac Makefile.am po/.gitignore po/LINGUAS po/ru.po

2013-12-02 Thread Lennart Poettering
On Fri, 29.11.13 14:45, Colin Walters (walt...@verbum.org) wrote:

> On Fri, 2013-11-29 at 02:39 +0100, Lennart Poettering wrote:
> > On Thu, 28.11.13 00:54, Zbigniew Jędrzejewski-Szmek 
> > (zbys...@kemper.freedesktop.org) wrote:
> 
> > Instead we should add some code to macro.h which turns off the the
> > warning with the #pragma stuff only if it detects it is being run on an
> > old gcc.
> 
> There's also this approach:

Note that C11 static_assert is explicitly allowed at the top-level
(heck, even inside of structs and unions!), so on modern systems we
should make use of that and not have to introduce a distinction there...

As it appears there are some gcc versions which claim to support C11
static assert but implement it so that compiler warnings are generated
if used otuside of blocks and with our sets of warnings set. My
suggestion is to turn off these warnings on those compiler versions (and
only on those compiler versions), and leave them on for new versions
where this is not an issue.

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [RFC 05/12] gfx: add sd-gfx library with unifont section

2013-12-02 Thread David Herrmann
Hi

>> The binary file contains all glyphs in a compressed 1-bit-per-pixel format
>> for all 8x16 and 16x16 glyphs. It is linked directly into libsystemd-gfx
>> via the *.bin makefile target. Binary size is 2.1MB, but thanks to paging,
>> the kernel only loads required pages into memory. A ASCII-only screen thus
>> only needs 40k VIRT mem.
> Do we have quad-glyphs (16x32) for the ASCII part of this for hi-res screens?
>
> (also, if we use NFD we could reuse these for some of unicode[1], but
> that is less important)
>
> [1]http://blog.golang.org/normalization

See here:

+static void gfx_glyph_blend(gfx_glyph *g, unsigned int ppi, const
struct unifont_data *u) {
+unsigned int i, j;
+const uint8_t *src;
+uint8_t *dst;
+
+/*
+ * TODO: scale glyph according to @ppi
+ */

Idea is to simply scale the glyph by an integer (sth like max(1, ppi /
72)). This will result in a readable font-size on all screens. Note
that we don't care for hi-res fonts, we're not EFI-firmware..

Regarding NFD: I don't care for normalization. I don't do any
string-comparisons. We support combining characters in the
font-renderer just fine, so I don't think we ever need unicode
normalization.

Thanks
David
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] [PATCH] Display synthetic message serial number in a more readable format than (uint32_t) -1

2013-12-02 Thread Lukasz Skalski
Serial=4294967295 field in message dump generated by bus_message_dump()
function for synthetic messages isn't good readable.
---
 src/libsystemd-bus/bus-dump.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/libsystemd-bus/bus-dump.c b/src/libsystemd-bus/bus-dump.c
index 469f7ba..71de081 100644
--- a/src/libsystemd-bus/bus-dump.c
+++ b/src/libsystemd-bus/bus-dump.c
@@ -56,15 +56,21 @@ int bus_message_dump(sd_bus_message *m, FILE *f, bool 
with_header) {
 
 if (with_header) {
 fprintf(f,
-"%s%s%sType=%s%s%s  Endian=%c  Flags=%u  Version=%u  
Serial=%u ",
+"%s%s%sType=%s%s%s  Endian=%c  Flags=%u  Version=%u",
 m->header->type == SD_BUS_MESSAGE_METHOD_ERROR ? 
ansi_highlight_red() :
 m->header->type == SD_BUS_MESSAGE_METHOD_RETURN ? 
ansi_highlight_green() :
 m->header->type != SD_BUS_MESSAGE_SIGNAL ? 
ansi_highlight() : "", draw_special_char(DRAW_TRIANGULAR_BULLET), 
ansi_highlight_off(),
 ansi_highlight(), 
bus_message_type_to_string(m->header->type), ansi_highlight_off(),
 m->header->endian,
 m->header->flags,
-m->header->version,
-BUS_MESSAGE_SERIAL(m));
+m->header->version);
+
+/* Display synthetic message serial number in a more readable
+ * format than (uint32_t) -1 */
+if (BUS_MESSAGE_SERIAL(m) == 0xULL)
+fprintf(f, " Serial=-1");
+else
+fprintf(f, " Serial=%u", BUS_MESSAGE_SERIAL(m));
 
 if (m->reply_serial != 0)
 fprintf(f, "  ReplySerial=%u", m->reply_serial);
-- 
1.8.3.2

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] [PATCH] Avoid a busy systemd-journald in LXC environments

2013-12-02 Thread Werner Fink
that is the systemd-journald may ignore /dec/kmsg which are not a valid
device but a lint to /dev/null as done in LinuX Containers (LXC).  This
change adds straight forward a check for /dev/kmsg.

Signed-off-by: Werner Fink 
---
 src/journal/journald-kmsg.c |   27 ++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git src/journal/journald-kmsg.c src/journal/journald-kmsg.c
index af49d85..03d8f90 100644
--- src/journal/journald-kmsg.c
+++ src/journal/journald-kmsg.c
@@ -23,7 +23,9 @@
 #include 
 #include 
 #include 
+#include 
 #include 
+#include 
 
 #include 
 #include 
@@ -377,12 +379,32 @@ int server_flush_dev_kmsg(Server *s) {
 
 int server_open_dev_kmsg(Server *s) {
 struct epoll_event ev;
+struct stat st;
 
 assert(s);
 
 s->dev_kmsg_fd = open("/dev/kmsg", 
O_RDWR|O_CLOEXEC|O_NONBLOCK|O_NOCTTY);
 if (s->dev_kmsg_fd < 0) {
-log_warning("Failed to open /dev/kmsg, ignoring: %m");
+/* Do not warn as it may not exists in LXC environments */
+if (errno != ENOENT)
+log_warning("Failed to open /dev/kmsg, ignoring: %m");
+return 0;
+}
+
+if (fstat(s->dev_kmsg_fd, &st) < 0) {
+log_error("Failed to stat /dev/kmsg fd, ignoring: %m");
+close_nointr_nofail(s->dev_kmsg_fd);
+s->dev_kmsg_fd = -1;
+return 0;
+}
+
+if (!S_ISCHR(st.st_mode) || major(st.st_rdev) != 1 || 
minor(st.st_rdev) != 11) {
+int old_errno = errno;
+errno = ENODEV;
+log_warning("Irregular device /dev/kmsg, ignoring: %m");
+errno = old_errno;
+close_nointr_nofail(s->dev_kmsg_fd);
+s->dev_kmsg_fd = -1;
 return 0;
 }
 
@@ -391,6 +413,9 @@ int server_open_dev_kmsg(Server *s) {
 ev.data.fd = s->dev_kmsg_fd;
 if (epoll_ctl(s->epoll_fd, EPOLL_CTL_ADD, s->dev_kmsg_fd, &ev) < 0) {
 
+close_nointr_nofail(s->dev_kmsg_fd);
+s->dev_kmsg_fd = -1;
+
 /* This will fail with EPERM on older kernels where
  * /dev/kmsg is not readable. */
 if (errno == EPERM)
-- 
1.7.9.2

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [RFC 05/12] gfx: add sd-gfx library with unifont section

2013-12-02 Thread Shawn Landden
On Wed, Nov 27, 2013 at 10:48 AM, David Herrmann  wrote:
> If we want to interact with a user in the initrd, during
> emergency-situations, in single-user mode, or in any other rather limited
> situation, we currently rely on the kernel to do input and graphics access
> for us. More precisely, we rely on the VT layer to do keyboard parsing and
> font rendering. Or in other words: The *kernel* provides our UI!
>
> This is bad for the same reasons we don't put any other UI into the
> kernel. However, there are a few reasons to keep a minimal UI in the
> kernel:
>  1) show panic-screen with kernel oops message
>  2) show log-screen during early boot
>  3) allow kernel-debugging via kdb
> While people using kdb are encouraged to keep VTs, for 1) and 2) there is
> a replacement via fblog/drmlog.
>
> So we run out of reasons to keep a UI in the kernel. However, to allow
> moving the UI handling to userspace, we need a bunch of helpers:
>  - keyboard handling: convert keycodes into keysyms and modifiers/..
>  - modesetting: program the gfx pipeline and provide a rendering
> infrastructure
>  - fonts: to render text, we need some basic fonts
>  - hotplugging: device detection and assignment during runtime
> (Note that all these are implemented (often quite rudimentary) in the
> kernel to allow a basic UI.)
>
> This patch introduces sd-gfx, a systemd-internal library dealing with all
> these things. Note that it is designed to be exported some day, but for
> now we keep it internal.
> While the library itself will be kept small and almost self-contained, it
> is designed to be extensible and can be used in rather complex graphics
> applications. For systemd, we plan to add a basic emergency-console, an
> initrd password-query, kernel-log-screen and a fallback login-screen.
> These allow booting without CONFIG_VT and provide a basic system for
> seats without VTs (either CONFIT_VT=n or seats != seat0).
>
> As a first step, we add the required header+build-chain and add the
> font-handling. To avoid heavy font-pipelines in systemd, we only provide
> a statically-sized fallback-font based on GNU-Unifont. It contains glyphs
> for the *whole* Base-Multilingual-Plane of Unicode and thus allows
> internationalized text.
>
> The "make-unifont.py" script is used by the "make update-unifont" custom
> target to regenerate the unifont files. As this can take quite some time,
> we check the result into git so only maintainers need to run this.
>
> The binary file contains all glyphs in a compressed 1-bit-per-pixel format
> for all 8x16 and 16x16 glyphs. It is linked directly into libsystemd-gfx
> via the *.bin makefile target. Binary size is 2.1MB, but thanks to paging,
> the kernel only loads required pages into memory. A ASCII-only screen thus
> only needs 40k VIRT mem.
Do we have quad-glyphs (16x32) for the ASCII part of this for hi-res screens?

(also, if we use NFD we could reuse these for some of unicode[1], but
that is less important)

[1]http://blog.golang.org/normalization
> ---
> Hi
>
> I removed the unifont-data from this patch so this will not apply cleanly.
> However, the ML would reject the huge patch otherwise. If anyone is interested
> in the raw patch, the series is available on:
>   http://cgit.freedesktop.org/~dvdhrm/systemd/log/?h=console
>
> Thanks
> David
>
>  Makefile.am  |31 +
>  configure.ac |10 +
>  make-unifont.py  |   138 +
>  src/libsystemd-gfx/.gitignore| 1 +
>  src/libsystemd-gfx/Makefile  | 1 +
>  src/libsystemd-gfx/gfx-unifont.c |   273 +
>  src/libsystemd-gfx/unifont.bin   |   Bin 0 -> 2162688 bytes
>  src/libsystemd-gfx/unifont.hex   | 63488 
> +
>  src/systemd/sd-gfx.h |65 +
>  9 files changed, 64007 insertions(+)
>  create mode 100755 make-unifont.py
>  create mode 100644 src/libsystemd-gfx/.gitignore
>  create mode 12 src/libsystemd-gfx/Makefile
>  create mode 100644 src/libsystemd-gfx/gfx-unifont.c
>  create mode 100644 src/libsystemd-gfx/unifont.bin
>  create mode 100644 src/libsystemd-gfx/unifont.hex
>  create mode 100644 src/systemd/sd-gfx.h
>
> diff --git a/Makefile.am b/Makefile.am
> index ce27e82..2edb091 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -3853,6 +3853,37 @@ EXTRA_DIST += \
>  endif
>
>  # 
> --
> +if HAVE_GFX
> +noinst_LTLIBRARIES += \
> +   libsystemd-gfx.la
> +
> +libsystemd_gfx_la_SOURCES = \
> +   src/libsystemd-gfx/sd-gfx.h \
> +   src/libsystemd-gfx/gfx-unifont.c
> +
> +libsystemd_gfx_la_CFLAGS = \
> +   $(AM_CFLAGS)
> +
> +libsystemd_gfx_la_LIBADD = \
> +   libsystemd-shared.la \
> +   src/libsystemd-gfx/unifont.bin.lo
> +
> +src/libsystemd-gfx/unifont.bin: make-unifont.py 
> src/libsystemd-gfx/unifont.hex
> +   $(AM_V_GEN)cat $(top_srcdir)/src/libsystemd-gfx/unifont.hex | 
> $(PYTHON) $< >$@
> +
> +src/lib

Re: [systemd-devel] Thread level resource management

2013-12-02 Thread Umut Tezduyar Lindskog
Hi,

> -Original Message-
> From: Lennart Poettering [mailto:lenn...@poettering.net]
> Sent: den 29 november 2013 13:49
> To: Umut Tezduyar Lindskog
> Cc: David Timothy Strauss; systemd-devel@lists.freedesktop.org; Kay Sievers
> Subject: Re: [systemd-devel] Thread level resource management
> 
> On Fri, 29.11.13 09:33, Umut Tezduyar Lindskog (umut.tezdu...@axis.com)
> wrote:
> 
> >
> > Hi,
> >
> > Find my comments below please.
> >
> > On Nov 29, 2013, at 9:51 AM, David Timothy Strauss
>  wrote:
> >
> > > On Fri, Nov 29, 2013 at 1:58 AM, Umut Tezduyar Lindskog
> > >  wrote:
> > >> Can someone explain the process level management?
> > >
> > > Right now, it's possible to do directly in the cgroups file system,
> > > but we're eventually moving away from anything manipulating that but
> > > systemd. I think that there will still be a way to move around
> > > processes via systemd, but it's speculation at this point.
> > I am after that “way” you are referring.
> 
> Well, you already can do some bits of it, like using
> pthread_setschedparam()/pthread_setschedprio() on individual threads.
> That will allow you to define different scheduling parameters for your
> threads. This does not allow grouping though. To provide for that there's
> probably going to be an interface in /proc/self/task/$TID/. At least that's
> what the kernel guys are discussing right now, if I understood things 
> correctly.
> 
> > > Using services will allow you to easily configure resources in a way
> > > that will continue working through 2014 and beyond as systemd and
> > > the kernel update. Even with separate services, you can still use
> > > multithreaded-style (shared memory) techniques by mmapping the
> same
> > > paths with MAP_SHARED. There are a bunch of other, standard IPC
> > > mechanisms, too [1]. It's generally best to decouple the program
> > > into services that communicate at a high level.
> 
> > I think it would be very nice to hear some roadmap on how it is going
> > to be. I am a bit worried in terms of when/how to start changing
> > current applications that have been making use of thread level
> > resource management like gstreamer.
> 
> Precisely what kind of thread level resource management is gst doing with
> cgroups?
Each stream is being handled in an isolated thread and threads cpu share is set 
by cgroups using thread's task id. 
> 
> Lennart
> 
> --
> Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [systemd-commits] 4 commits - catalog/systemd-fr.catalog catalog/systemd-ru.catalog configure.ac Makefile.am po/.gitignore po/LINGUAS po/ru.po

2013-12-02 Thread Lucas De Marchi
On Thu, Nov 28, 2013 at 11:39 PM, Lennart Poettering
 wrote:
> On Thu, 28.11.13 00:54, Zbigniew Jędrzejewski-Szmek 
> (zbys...@kemper.freedesktop.org) wrote:
>
>> commit f1a1264d13b31b9f5521f482d9a5a9d78da55efb
>> Author: Zbigniew J??drzejewski-Szmek 
>> Date:   Thu Nov 28 03:41:33 2013 -0500
>>
>> build-sys: avoid warnings from assert_cc
>>
>> diff --git a/configure.ac b/configure.ac
>> index c0656f4..f1b00c5 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -128,7 +128,6 @@ CC_CHECK_FLAGS_APPEND([with_cflags], [CFLAGS], [\
>>  -Wold-style-definition \
>>  -Wpointer-arith \
>>  -Winit-self \
>> --Wdeclaration-after-statement \
>>  -Wfloat-equal \
>>  -Wmissing-prototypes \
>>  -Wstrict-prototypes \
>>
>
> I reverted this bit. Newer GCC versions don't suffer by this problem,
> and I think the warning makes a lot of sense on those.
>
> Instead we should add some code to macro.h which turns off the the
> warning with the #pragma stuff only if it detects it is being run on an
> old gcc.
>
> I would have just added this on my own, but I can't test this, since my
> gcc is new enough to not need this.
>
> gcc 4.8.2 (which is what Fedora 20 is using) works fine. So I would
> suggest some ifdef check in macro.h that uses #pragma to globally turn
> off the warning if anything < 4.8.2 i used...

Actually with anything newer or equal to gcc 4.6 we should be using
_Static_assert that's C11. systemd already checks if "static_assert"
is defined, which should work fine. I'm not sure if it's being
disabled depending on the build options due to the check in assert.h
(in Arch i'm getting the C11 behavior):

#if defined __USE_ISOC11 && !defined __cplusplus
/* Static assertion.  Requires support in the compiler.  */
# undef static_assert
# define static_assert _Static_assert
#endif

In kmod I'm checking by _Static_assert availability in configure and
defining a slightly different assert_cc macro for older compilers,
which appears to not show any warnings there, too.

Lucas De Marchi
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] The whole su/pkexec session debate

2013-12-02 Thread David Herrmann
Hi

On Mon, Dec 2, 2013 at 11:00 AM, Colin Guthrie  wrote:
> 'Twas brillig, and Martin Pitt at 02/12/13 05:48 did gyre and gimble:
>>> > This way, screen will keep an "active" reference to the session and
>>> > systemd-logind will not mark it as "closing".
>>
>> But that screen process would still be running in the user's logind
>> session cgroup, so logind can see that the session is still active
>> that way? (Unless you configured it to kill all session processes on
>> logout).
>
> The session is still marked as "closing" but because processes still
> exist it never quite dies. And yes, the kill processes option (which is
> a nice thing to enable if possible) would indeed kill the screen.

We have this nice "pipe" which is leaked into every session-process.
If they fork(), the pipe is dup()ed (unless they use /proc/self/fd/*
to close all fds) into the child. Only when *all* these processes are
dead, logind gets HUP on its side of the pipe.

Unfortunately, we make pam_close_session() call ReleaseSession() which
instructs logind to mark the session as "closing" and disables the
pipe-logic.

So to fix this, we could remove the ReleaseSession() call from
pam_systemd.c and solely rely on the pipe-logic. This however,
prevents normal gnome-session sessions from being marked as "closing"
once the session exited but some random process lingers in the
session-cgroup.
But then gnome-session should simply call ReleaseSession() on the bus itself..

Cheers
David
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [RFC] logind: introduce session "positions"

2013-12-02 Thread David Herrmann
Hi

On Mon, Dec 2, 2013 at 10:57 AM, Colin Guthrie  wrote:
> 'Twas brillig, and David Herrmann at 01/12/13 11:43 did gyre and gimble:
>> During session_load() or if two sessions have the same VTnr, we may end up
>> with two sessions with the same position (this shouldn't happen, but lets
>> be fail-safe in case some other part of the stack fails).
>
> I presume this would actually be quite easy if we get around to
> implementing Lennart's suggested force-new-session=1 pam_systemd option
> which will be used by su-l, sudo-i, and possible polkit stuff?

Not really. On seats without VTs, the new session just ends up as a
new session (with it's own position or no position at all if
class=="background" and we skip positions for background sessions).

On seats with VTs, it really doesn't matter as SwitchTo() just results
in chvt(). So behavior stays the same.
You're right that on VTs we can easily end up with two sessions on a
single VT. I dislike that, but apparently people want it.. You get
very weird semantics then, but I'm not touching that code as I don't
care.

>> This case is
>> dealt with gracefully by ignoring any session but the first session
>> assigned to the position. Thus, session->pos is a hint, seat->positions[i]
>> is the definite position-assignment. Always verify both match in case you
>> need to modify them!
>
> Even if pam_systemd did get this new feature, it sounds like this
> strategy would work fine for dealing with switching anyway.

Yepp, I designed the fallback to treat duplicates as
class==background. So this should be fine.

Thanks
David
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [RFC] logind: introduce session "positions"

2013-12-02 Thread David Herrmann
Hi

On Sun, Dec 1, 2013 at 6:02 PM, Shawn Landden  wrote:
> On Sun, Dec 1, 2013 at 3:43 AM, David Herrmann  wrote:
>> logind has no concept of session ordering. Sessions have a unique name,
>> some attributes about the capabilities and that's already it. There is
>> currently no stable+total order on sessions. If we use the logind API to
>> switch between sessions, we are faced with an unordered list of sessions
>> we have no clue of.
>>
>> This used to be no problem on seats with VTs or on seats with only a
>> single active session. However, with the introduction of multi-session
>> capability for seats without VTs, we need to find a way to order sessions
>> in a stable way.
>>
>> This patch introduces session "positions". A position is a simple integer
>> assigned to a session which is never changed implicitly (currently, we
>> also don't change it explicitly, but that may be changed someday). For
>> seats with VTs, we force the position to be the same as the VTnr. Without
>> VTs, we simply find the lowest unassigned number and use it as position.
>> If position-assignment fails or if, for any reason, we decide to not
>> assign a position to a session, the position is set to 0 (which is treated
>> as invalid position).
>> During session_load() or if two sessions have the same VTnr, we may end up
>> with two sessions with the same position (this shouldn't happen, but lets
>> be fail-safe in case some other part of the stack fails). This case is
>> dealt with gracefully by ignoring any session but the first session
>> assigned to the position. Thus, session->pos is a hint, seat->positions[i]
>> is the definite position-assignment. Always verify both match in case you
>> need to modify them!
>>
>> Additionally, we introduce SwitchTo(unsigned int) on the seat-dbus-API.
>> You can call it with any integer value != 0 and logind will try to switch
>> to the request position. If you implement a compositor or any other
>> session-controller, you simply watch for ctrl+alt+F1 to F12 and call
>> SwitchTo(Fx). logind will figure a way out deal with this number.
>> For convenience, we also introduce SwitchToNext/Previous(). It should be
>> called on ctrl+alt+Left/Right (like the kernel-console used to support).
> This has some conflict with workspaces, but not in gnome-shell (w/o
> gnome-tweak-tool)
> as there workspaces are all vertical. I personally like the idea.

Umm, right. These keys are already used by some compositors. But we
would have to introduce XKB_KEY_XF86Switch_To_Next/Previous, anyway.
So these actions are not bound to any shortcuts but it depends on your
keymap. This is the same as with XKB_KEY_XF86Switch_To_VTx which are
also bound by your keymap, not by your compositor.

>>
>> Note that the public API (SwitchTo*()) is *not* bound to the underlying
>> logic that is implemented now. We don't export "session-positions" on the
>> dbus/C API! They are an implementation detail. Instead, the SwitchTo*()
>> API is supposed to be a hint to let logind choose the session-switching
>> logic. Any foreground session-controller is free to enumerate/order
>> existing sessions according to their needs and call Session.Activate()
>> manually. But the SwitchTo*() API provides a uniform behavior across
>> session-controllers.
>>
>> Background: Session-switching keys depend on the active keymap. The XKB
>> specification provides the XKB_KEY_XF86Switch_VT_1-12 key-symbols which
>> have to be mapped by all keymaps to allow session-switching. It is usually
>> bound to ctrl+alt+Fx but may be set differently. A compositor passes any
>> keyboard input to XKB before passing it to clients. In case a key-press
>> invokes the XKB_KEY_XF86Switch_VT_x action, the keypress is *not*
>> forwarded to clients, but instead a session-switch is scheduled.
>>
>> This actually prevents us from handling these keys outside of the session.
>> If an active compositor has a keymap with a different mapping of these
>> keys, and logind itself tries to catch these combinations, we end up with
>> the key-press sent to the compositor's clients *and* handled by logind.
>> This is *bad* and we must avoid this. The only situation where a
>> background process is allowed to handle key-presses is debugging and
>> emergency-keys. In these cases, we don't care for keymap mismatches and
>> accept the double-event. Another exception is unmapped keys like
>> PowerOff/Suspend (even though this one is controversial).
>>
>> Future ideas: As this commit-msg isn't long enough, yet, some notes on
>> future ideas. The current position-assignment is compatible with the
>> legacy VT numbers. However, it is a rather outdated way of addressing
>> sessions. Instead, we can make use of session-classes of logind. We
>> already tag session with one of the classes "greeter", "user",
>> "background", "lock-screen". So one of my ideas is to make
>> "position-assignment" a "per-class" thing. And instead of mapping F1-F12
>> directly to the positions, we map it as follows:
>>  - F

Re: [systemd-devel] [RFC] logind: introduce session "positions"

2013-12-02 Thread David Herrmann
Hi

On Mon, Dec 2, 2013 at 1:09 PM, Tom Gundersen  wrote:
> On Sun, Dec 1, 2013 at 12:43 PM, David Herrmann  wrote:
>> logind has no concept of session ordering. Sessions have a unique name,
>> some attributes about the capabilities and that's already it. There is
>> currently no stable+total order on sessions. If we use the logind API to
>> switch between sessions, we are faced with an unordered list of sessions
>> we have no clue of.
>>
>> This used to be no problem on seats with VTs or on seats with only a
>> single active session. However, with the introduction of multi-session
>> capability for seats without VTs, we need to find a way to order sessions
>> in a stable way.
>>
>> This patch introduces session "positions". A position is a simple integer
>> assigned to a session which is never changed implicitly (currently, we
>> also don't change it explicitly, but that may be changed someday). For
>> seats with VTs, we force the position to be the same as the VTnr. Without
>> VTs, we simply find the lowest unassigned number and use it as position.
>> If position-assignment fails or if, for any reason, we decide to not
>> assign a position to a session, the position is set to 0 (which is treated
>> as invalid position).
>> During session_load() or if two sessions have the same VTnr, we may end up
>> with two sessions with the same position (this shouldn't happen, but lets
>> be fail-safe in case some other part of the stack fails). This case is
>> dealt with gracefully by ignoring any session but the first session
>> assigned to the position. Thus, session->pos is a hint, seat->positions[i]
>> is the definite position-assignment. Always verify both match in case you
>> need to modify them!
>>
>> Additionally, we introduce SwitchTo(unsigned int) on the seat-dbus-API.
>> You can call it with any integer value != 0 and logind will try to switch
>> to the request position. If you implement a compositor or any other
>> session-controller, you simply watch for ctrl+alt+F1 to F12 and call
>> SwitchTo(Fx). logind will figure a way out deal with this number.
>> For convenience, we also introduce SwitchToNext/Previous(). It should be
>> called on ctrl+alt+Left/Right (like the kernel-console used to support).
>
> So if (somehow) two sessions are assigned to the same position, the

This "somehow" really means someone hand-edited
/run/systemd/sessions/* or you have a corrupted FS and then
systemd-logind crashed. You never end up with two sessions on the same
position under normal conditions. I just mentioned it so people better
understand the error/fallback paths in the code.

This obviously isn't true for seats with VTs. There you can start
multiple sessions on the same VT. However, this is strongly
discouraged as systemd randomly chooses one session to be active then.

> behavior of Next/Previous would be something like: if the 'first'
> session on the given position is active, and you do SwitchToNext
> followed by SwitchToPrevious you get back were you started. However,
> if the 'second' session is the active one and you do Next/Previous you
> end up at the 'first' one?
>
> If we want to ensure that Next/Previous are true inverses of each
> other (which I guess might be more intuitive?) we could simply say
> that the most recent session on a given position is the 'first' one,
> does that make sense? Of course that means that Session.Activate() may
> change the session associated with, say, SwitchTo(3), but reading the
> stuff you wrote below about classes, that sort of makes sense.
>
> Also, what is the semantics of position 0, are these truly unordered,
> so that Next/Previous will fail when called on them, or will you end
> up at the 'first'/'last' position respectively? (Looking at the code,
> it seems that SwitchToNext is defined for position 0, but I may have
> misread).

Position 0 is unused. SwitchTo(0) returns EINVAL and
SwitchToNext/Previous skip this position. This is copied from VTs
where pos==0 is invalid, too.

>> Note that the public API (SwitchTo*()) is *not* bound to the underlying
>> logic that is implemented now. We don't export "session-positions" on the
>> dbus/C API! They are an implementation detail. Instead, the SwitchTo*()
>> API is supposed to be a hint to let logind choose the session-switching
>> logic. Any foreground session-controller is free to enumerate/order
>> existing sessions according to their needs and call Session.Activate()
>> manually. But the SwitchTo*() API provides a uniform behavior across
>> session-controllers.
>>
>> Background: Session-switching keys depend on the active keymap. The XKB
>> specification provides the XKB_KEY_XF86Switch_VT_1-12 key-symbols which
>> have to be mapped by all keymaps to allow session-switching. It is usually
>> bound to ctrl+alt+Fx but may be set differently. A compositor passes any
>> keyboard input to XKB before passing it to clients. In case a key-press
>> invokes the XKB_KEY_XF86Switch_VT_x action, the keypress is *not*
>> forwarded to

Re: [systemd-devel] [RFC] logind: introduce session "positions"

2013-12-02 Thread Tom Gundersen
On Sun, Dec 1, 2013 at 12:43 PM, David Herrmann  wrote:
> logind has no concept of session ordering. Sessions have a unique name,
> some attributes about the capabilities and that's already it. There is
> currently no stable+total order on sessions. If we use the logind API to
> switch between sessions, we are faced with an unordered list of sessions
> we have no clue of.
>
> This used to be no problem on seats with VTs or on seats with only a
> single active session. However, with the introduction of multi-session
> capability for seats without VTs, we need to find a way to order sessions
> in a stable way.
>
> This patch introduces session "positions". A position is a simple integer
> assigned to a session which is never changed implicitly (currently, we
> also don't change it explicitly, but that may be changed someday). For
> seats with VTs, we force the position to be the same as the VTnr. Without
> VTs, we simply find the lowest unassigned number and use it as position.
> If position-assignment fails or if, for any reason, we decide to not
> assign a position to a session, the position is set to 0 (which is treated
> as invalid position).
> During session_load() or if two sessions have the same VTnr, we may end up
> with two sessions with the same position (this shouldn't happen, but lets
> be fail-safe in case some other part of the stack fails). This case is
> dealt with gracefully by ignoring any session but the first session
> assigned to the position. Thus, session->pos is a hint, seat->positions[i]
> is the definite position-assignment. Always verify both match in case you
> need to modify them!
>
> Additionally, we introduce SwitchTo(unsigned int) on the seat-dbus-API.
> You can call it with any integer value != 0 and logind will try to switch
> to the request position. If you implement a compositor or any other
> session-controller, you simply watch for ctrl+alt+F1 to F12 and call
> SwitchTo(Fx). logind will figure a way out deal with this number.
> For convenience, we also introduce SwitchToNext/Previous(). It should be
> called on ctrl+alt+Left/Right (like the kernel-console used to support).

So if (somehow) two sessions are assigned to the same position, the
behavior of Next/Previous would be something like: if the 'first'
session on the given position is active, and you do SwitchToNext
followed by SwitchToPrevious you get back were you started. However,
if the 'second' session is the active one and you do Next/Previous you
end up at the 'first' one?

If we want to ensure that Next/Previous are true inverses of each
other (which I guess might be more intuitive?) we could simply say
that the most recent session on a given position is the 'first' one,
does that make sense? Of course that means that Session.Activate() may
change the session associated with, say, SwitchTo(3), but reading the
stuff you wrote below about classes, that sort of makes sense.

Also, what is the semantics of position 0, are these truly unordered,
so that Next/Previous will fail when called on them, or will you end
up at the 'first'/'last' position respectively? (Looking at the code,
it seems that SwitchToNext is defined for position 0, but I may have
misread).

> Note that the public API (SwitchTo*()) is *not* bound to the underlying
> logic that is implemented now. We don't export "session-positions" on the
> dbus/C API! They are an implementation detail. Instead, the SwitchTo*()
> API is supposed to be a hint to let logind choose the session-switching
> logic. Any foreground session-controller is free to enumerate/order
> existing sessions according to their needs and call Session.Activate()
> manually. But the SwitchTo*() API provides a uniform behavior across
> session-controllers.
>
> Background: Session-switching keys depend on the active keymap. The XKB
> specification provides the XKB_KEY_XF86Switch_VT_1-12 key-symbols which
> have to be mapped by all keymaps to allow session-switching. It is usually
> bound to ctrl+alt+Fx but may be set differently. A compositor passes any
> keyboard input to XKB before passing it to clients. In case a key-press
> invokes the XKB_KEY_XF86Switch_VT_x action, the keypress is *not*
> forwarded to clients, but instead a session-switch is scheduled.
>
> This actually prevents us from handling these keys outside of the session.
> If an active compositor has a keymap with a different mapping of these
> keys, and logind itself tries to catch these combinations, we end up with
> the key-press sent to the compositor's clients *and* handled by logind.
> This is *bad* and we must avoid this. The only situation where a
> background process is allowed to handle key-presses is debugging and
> emergency-keys. In these cases, we don't care for keymap mismatches and
> accept the double-event. Another exception is unmapped keys like
> PowerOff/Suspend (even though this one is controversial).
>
> Future ideas: As this commit-msg isn't long enough, yet, some notes on
> future ideas. The current

Re: [systemd-devel] The whole su/pkexec session debate

2013-12-02 Thread Colin Guthrie
'Twas brillig, and Martin Pitt at 02/12/13 05:48 did gyre and gimble:
>> > This way, screen will keep an "active" reference to the session and
>> > systemd-logind will not mark it as "closing".
>
> But that screen process would still be running in the user's logind
> session cgroup, so logind can see that the session is still active
> that way? (Unless you configured it to kill all session processes on
> logout).

The session is still marked as "closing" but because processes still
exist it never quite dies. And yes, the kill processes option (which is
a nice thing to enable if possible) would indeed kill the screen.

It would be really nice if screen somehow escaped, but if the pam* calls
need root then I think some other way would be better (perhaps with
logind doing some of the setup work... dunno).

Col

-- 

Colin Guthrie
gmane(at)colin.guthr.ie
http://colin.guthr.ie/

Day Job:
  Tribalogic Limited http://www.tribalogic.net/
Open Source:
  Mageia Contributor http://www.mageia.org/
  PulseAudio Hacker http://www.pulseaudio.org/
  Trac Hacker http://trac.edgewall.org/
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [RFC] logind: introduce session "positions"

2013-12-02 Thread Colin Guthrie
'Twas brillig, and David Herrmann at 01/12/13 11:43 did gyre and gimble:
> During session_load() or if two sessions have the same VTnr, we may end up
> with two sessions with the same position (this shouldn't happen, but lets
> be fail-safe in case some other part of the stack fails). 

I presume this would actually be quite easy if we get around to
implementing Lennart's suggested force-new-session=1 pam_systemd option
which will be used by su-l, sudo-i, and possible polkit stuff?

> This case is
> dealt with gracefully by ignoring any session but the first session
> assigned to the position. Thus, session->pos is a hint, seat->positions[i]
> is the definite position-assignment. Always verify both match in case you
> need to modify them!

Even if pam_systemd did get this new feature, it sounds like this
strategy would work fine for dealing with switching anyway.

Col


-- 

Colin Guthrie
gmane(at)colin.guthr.ie
http://colin.guthr.ie/

Day Job:
  Tribalogic Limited http://www.tribalogic.net/
Open Source:
  Mageia Contributor http://www.mageia.org/
  PulseAudio Hacker http://www.pulseaudio.org/
  Trac Hacker http://trac.edgewall.org/
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel