Re: [systemd-devel] Apparmor profile switching support

[Zbigniew Jędrzejewski-Szmek]

On Fri, Jan 03, 2014 at 05:58:46PM +0100, Michael Scherer wrote:
> Le vendredi 03 janvier 2014 à 17:22 +0100, m...@zarb.org a écrit :
> > As discussed on the SELinux thread, this patch attempt to offer the same
> > level of configuration for Apparmor distributions by permitting to the
> > sysadmin to set the profile used by a unit. I didn't tested it but would 
> > like to get early feedback on it from openSUSE and Ubuntu users, as they
> > are the 2 main set of users of AppArmor.
> > 
> > Main inspiration come from the upstart support, on 
> > https://code.launchpad.net/~mdeslaur/upstart/apparmor-support
> > However, we are currently lacking the capacity of using directly a on disk 
> > profile, and
> > I am not sure on the best way to support that. 
> 
> I have also been told on irc that Michael Stapelberg wrote the same kind
> of patch ( if not the same, given there isn't much possible variation ),
> cf https://lists.debian.org/debian-security/2014/01/msg8.html

Your patch looks fine. I sent a comment on the patch 1/2 in the other mail.
Even though it's very simple it would be great if you could test it after
proposed changes. If nobody objects, I'd merge this in a few days.

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

Re: [systemd-devel] [PATCH 1/2] Add switch_apparmor_profile helper, to switch the profile of the next command to run. This can be used to load a custom apparmor profile for a unit.

[Zbigniew Jędrzejewski-Szmek]

On Fri, Jan 03, 2014 at 05:22:42PM +0100, m...@zarb.org wrote:
> From: Michael Scherer 
> 
> ---
>  src/shared/apparmor-util.c | 15 +++
>  src/shared/apparmor-util.h |  1 +
>  2 files changed, 16 insertions(+)
> 
> diff --git a/src/shared/apparmor-util.c b/src/shared/apparmor-util.c
> index 2b85da1..a75bec4 100644
> --- a/src/shared/apparmor-util.c
> +++ b/src/shared/apparmor-util.c
> @@ -39,3 +39,18 @@ bool use_apparmor(void) {
>  
>  return use_apparmor_cached;
>  }
> +
> +int switch_apparmor_profile(const char * profile) {
> +_cleanup_free_ char *filename = NULL;
> +_cleanup_fclose_ FILE *proc = NULL;
> +
> +if (asprintf (&filename, "/proc/%d/attr/exec", getpid()) <0)
> +return -ENOMEM;
> +
> +proc = fopen (filename, "w");
> +if (! proc)
> +return -errno;
> +
> +fprintf (proc, "exec %s\n", profile);
> +return 0;
> +}
This should be something like

int apparmor_switch_profile(const char *profile) {
char *p, *t;

p = procfs_file_alloca(0, "attr/exec");
t = strappenda("exec ", profile);

return write_string_file(p, t);
}

Totally untested, but there's no unnecessary malloc, and there's
a meaningful error returned if the thing most likely to fail, i.e. the
write, actually fails.

> diff --git a/src/shared/apparmor-util.h b/src/shared/apparmor-util.h
> index 4b056a1..f27608d 100644
> --- a/src/shared/apparmor-util.h
> +++ b/src/shared/apparmor-util.h
> @@ -24,3 +24,4 @@
>  #include 
>  
>  bool use_apparmor(void);
> +int switch_apparmor_profile(const char * profile);
   const char *profile

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

Re: [systemd-devel] [WIP][RFC][PATCH] networkd: generate resolv.conf

[Tom Gundersen]

Hi Djalal,

On Mon, Jan 6, 2014 at 12:17 AM, Djalal Harouni  wrote:
>>  1) nameservers receieved over DHCP
>>  2) nameservers statically configured in a currently active .network file
>>  3) nameservers statically configured in the global networkd configuration 
>> file

[...]

>> So first question: do you agree with adding this functionality?
>>
>> Second question: do you agree with the order of precedence I listed above?
> I'll comment on this:
>
> IMO by default try to not be so intrusive

What in particular do you find intrusive?

>  unless *told* so, some configs
> need their prefered/trusted nameservers.

With the current patch the admin (or packager) needs to add the
/etc/resolv.conf -> /run/systemd/network/resolv.conf symlink to get
any of this at all. If we assume this symlink is in place, I intend to
still make sure the admin has the full power by having the option of
statically configuring global and per-NIC nameservers (though these
will still apply globally due to how resolv.conf works). I'll also add
an option to opt-out of using the DNS servers received over DHCP.

Does that sound ok to you?

Cheers,

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

Re: [systemd-devel] [WIP][RFC][PATCH] networkd: generate resolv.conf

[Djalal Harouni]

Hi Tom,

On Sun, Jan 05, 2014 at 11:49:33PM +0100, Tom Gundersen wrote:
> This adds support to generate a basic resolv.conf in /run/systemd/network.
> This file will not take any effect unless the admin makes a symlink from
> /etc/resolv.conf.
> 
> The precedence of nameservers is:
> 
>  1) nameservers receieved over DHCP
>  2) nameservers statically configured in a currently active .network file
>  3) nameservers statically configured in the global networkd configuration 
> file
> 
> Note: /etc/resolv.conf is severely limited, so in the future we will likely
> rather provide a much more powerfull nss plugin (or something to that effect),
> but this should allow current users to function without any loss of
> functionality.
> ---
> 
> Hi guys,
> 
> This patch is not finished yet (currently only hooked up to DHCP, not to 
> statically
> configured addresses), but I thought I'd ask for some input early on.
> 
> It seems pretty clear that resolv.conf sucks, and we want somethig better (and
> that pretty soon). However, it seems simple to provide the legacy resolv.conf
> functionality at least until we have something better in place. Without it, I
> guess it will be a bit of a hassle to test networkd.
> 
> So first question: do you agree with adding this functionality?
> 
> Second question: do you agree with the order of precedence I listed above?
I'll comment on this:

IMO by default try to not be so intrusive unless *told* so, some configs
need their prefered/trusted nameservers.

Thanks

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

[systemd-devel] [WIP][RFC][PATCH] networkd: generate resolv.conf

[Tom Gundersen]

This adds support to generate a basic resolv.conf in /run/systemd/network.
This file will not take any effect unless the admin makes a symlink from
/etc/resolv.conf.

The precedence of nameservers is:

 1) nameservers receieved over DHCP
 2) nameservers statically configured in a currently active .network file
 3) nameservers statically configured in the global networkd configuration file

Note: /etc/resolv.conf is severely limited, so in the future we will likely
rather provide a much more powerfull nss plugin (or something to that effect),
but this should allow current users to function without any loss of
functionality.
---

Hi guys,

This patch is not finished yet (currently only hooked up to DHCP, not to 
statically
configured addresses), but I thought I'd ask for some input early on.

It seems pretty clear that resolv.conf sucks, and we want somethig better (and
that pretty soon). However, it seems simple to provide the legacy resolv.conf
functionality at least until we have something better in place. Without it, I
guess it will be a bit of a hassle to test networkd.

So first question: do you agree with adding this functionality?

Second question: do you agree with the order of precedence I listed above?

Cheers,

Tom

 Makefile.am|  2 ++
 src/network/networkd-link.c| 23 +
 src/network/networkd-manager.c | 74 ++
 src/network/networkd-network.c |  6 
 src/network/networkd.h |  5 +++
 5 files changed, 110 insertions(+)

diff --git a/Makefile.am b/Makefile.am
index 069583c..b1c40e0 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -4057,6 +4057,7 @@ systemd_networkd_LDADD = \
libsystemd-id128-internal.la \
libsystemd-rtnl.la \
libsystemd-dhcp.la \
+   libsystemd-label.la \
libsystemd-shared.la
 
 nodist_systemunit_DATA += \
@@ -4083,6 +4084,7 @@ test_network_LDADD = \
libsystemd-daemon-internal.la \
libsystemd-rtnl.la \
libsystemd-dhcp.la \
+   libsystemd-label.la \
libsystemd-shared.la
 
 tests += \
diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c
index bc8ca21..6c02782 100644
--- a/src/network/networkd-link.c
+++ b/src/network/networkd-link.c
@@ -310,6 +310,7 @@ static void dhcp_handler(sd_dhcp_client *client, int event, 
void *userdata) {
 struct in_addr address;
 struct in_addr netmask;
 struct in_addr gateway;
+struct in_addr nameserver;
 int prefixlen;
 int r;
 
@@ -403,6 +404,28 @@ static void dhcp_handler(sd_dhcp_client *client, int 
event, void *userdata) {
 addr = NULL;
 rt = NULL;
 
+r = sd_dhcp_client_get_dns(client, &nameserver);
+if (r >= 0) {
+_cleanup_address_free_ Address *dns = NULL;
+
+r = address_new_dynamic(&dns);
+if (r < 0) {
+log_error("Could not allocate DNS address");
+link_enter_failed(link);
+return;
+}
+
+dns->family = AF_INET;
+dns->in_addr.in = nameserver;
+
+link->dns = dns;
+dns = NULL;
+
+r = manager_update_resolv_conf(link->manager);
+if (r < 0)
+log_error("Failed to update resolv.conf");
+}
+
 link_enter_set_addresses(link);
 }
 
diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c
index 4e2cf45..0e59e3e 100644
--- a/src/network/networkd-manager.c
+++ b/src/network/networkd-manager.c
@@ -19,10 +19,13 @@
   along with systemd; If not, see .
  ***/
 
+#include 
+
 #include "path-util.h"
 #include "networkd.h"
 #include "libudev-private.h"
 #include "udev-util.h"
+#include "mkdir.h"
 
 int manager_new(Manager **ret) {
 _cleanup_manager_free_ Manager *m = NULL;
@@ -120,6 +123,12 @@ int manager_load_config(Manager *m) {
 if (r < 0)
 return r;
 
+if (m->dns) {
+r = manager_update_resolv_conf(m);
+if (r < 0)
+return r;
+}
+
 return 0;
 }
 
@@ -281,3 +290,68 @@ int manager_rtnl_listen(Manager *m) {
 
 return 0;
 }
+
+static void append_dns(FILE *f, Address *dns, unsigned *count) {
+char buf[INET6_ADDRSTRLEN];
+const char *address;
+
+address = inet_ntop(dns->family, &dns->in_addr, buf, INET6_ADDRSTRLEN);
+if (!address) {
+log_warning("Invalid DNS address. Ignoring.");
+return;
+}
+
+if (*count >= MAXNS) {
+log_warning("Too many configured name servers, ignoring 
'%s'.", address);
+return;
+}
+
+   

Re: [systemd-devel] [PATCH] core: support Distribute=n to distribute to n workers

[Zbigniew Jędrzejewski-Szmek]

On Thu, Dec 19, 2013 at 12:21:26PM -0800, Shawn Landden wrote:
> On Fri, Dec 13, 2013 at 8:23 PM, Shawn Landden  wrote:
> > If Distribute=n, turns SO_REUSEPORT on, and spawns
> > n workers to handling incoming requests.
> >
> > SO_REUSEPORT sockets on the same port must all be created
> > by the same uid, therefore using the option allows
> > other root programs (or programs of the same user
> > if running in --user mode) to "hijack" this port, even
> > after systemd reserves it.
> >
> > We spawn workers at a rate approximentally reverse
> > exponentially proportianal to the number of incoming connections.
> > Faster based on the time for new workers to start accept()ing
> > and their load, or slower if systemd is under load.
Hi Shawn,
sorry for the delay.

Your patch is nice, but I found three issues:

1. The documentation is still lacking. I made a small patch which extends
   and clarifies the description of Distribute=n a bit, but I think that
   even more explanation should be given [1]. Maybe you fold it into your
   patch?

2. It is possible that the instance name might be taken. One legitimate
   case would be when the socket is started, some instances are created,
   and the socket is stopped and started again. Then the connection count
   will be reset to 0. The user might also start an instance by hand. Such
   situations should not prevent the connection from being accepted.
   Something similar happens when snapshots are created, and systemd
   loops looking for a free name. The same fallback should be implemented
   here, either with linearly increasing instances, or maybe with random
   numbers in case the instance names is occupied.

3. The strategy of dup()ing the socket doesn't work. I wrote
   a simple server in python which logs the connections [2], and hooked
   it up into systemd [3-4] (*). If REUSEPORT was working correctly,
   each connection would be handled by just one instance, either created
   previously, or newly created by systemd for this connection. But
   I see the same connection being accept()ed by one of the instances
   and systemd itself spawning a new instance. I'm pretty sure that what
   Lennart wrote before, that you need to create a new socket bound to
   the same port for REUSEPORT to take effect, is true.

[1] 
http://in.waw.pl/~zbyszek/distribute-n/0001-Fix-Distribute-n-documentation.patch
[2] http://in.waw.pl/~zbyszek/distribute-n/socket_logger.py
[3] http://in.waw.pl/~zbyszek/distribute-n/distributed.socket
[4] http://in.waw.pl/~zbyszek/distribute-n/distributed@.service

(*) In the python script, it seems that print() statements don't reach
the journal, but systemd.journal.send()s do. I guess I'm missing something.
But that's why logging is duplicted. If somebody could explain this,
that would be great.
   
Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [RFC][PATCH] net: set default DEVTYPE for all ethernet based devices

[Tom Gundersen]

On Sun, Jan 5, 2014 at 8:44 PM, Marcel Holtmann  wrote:
>> Hm, I thought that was to fix false negatives. I.e., that some devices
>> with, say, DEVTYPE=bluetooth should in fact be treated as regular
>> ethernet?
>
> DEVTYPE=bluetooth and DEVTYPE=wlan are not false negatives. They should be 
> like that. That is something current userspace relies on. We can not break 
> this.
>
> However you will see a DEVTYPE=bluetooth as Ethernet emulation and another as 
> raw IP (6loWPAN). You want to differentiate these for the default address 
> assignment you pick. For example 6loWPAN will just rely on IPv6 route 
> solicitation.
>
> Same applies to DEVTYPE=wwan btw. Some Qualcomm cards can be exposed as 
> Ethernet or they do a raw IP only header in their netdev. DEVTYPE is just to 
> figure out the underlying technology of your network device. Then the ARPHRD_ 
> is to get an idea of the actual encapsulation that is used.

Thanks for the explanation. This fits what my expectations. So what we
are interested in is purely to match on the technology, and not on the
encapsulation. That would have to be a separate key, if it turns out
to be interesting (which it may very well do).

Cheers,

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

Re: [systemd-devel] [RFC][PATCH] net: set default DEVTYPE for all ethernet based devices

[Marcel Holtmann]

Hi Tom,

> In systemd's networkd and udevd, we would like to give the administrator a
> simple way to filter ethernet devices by their DEVTYPE. In order to avoid
> having a special treatment of the case where DEVTYPE=(null), initialize 
> it to
> a default value, "ethernet", in the kernel.
> 
> Signed-off-by: Tom Gundersen 
> Cc: Marcel Holtmann 
> Cc: Greg KH 
> ---
> 
> Hi Greg and Marcel,
> 
> This patch seems to do the right thing for me. Any comments before I send 
> it
> off to LKML?
> 
> I suppose it may make sense to hide this behind a kernel option in case 
> we are
> worried about breaking existing users (but if ConnMan is adapted, I don't 
> know
> of any other issues, NetworkManager is not affected at least).
> 
> Cheers,
> 
> Tom
> 
> net/core/dev.c | 6 ++
> 1 file changed, 6 insertions(+)
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index ba3b7ea..62881e0 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -6215,6 +6215,10 @@ void netdev_freemem(struct net_device *dev)
> kfree(addr);
> }
> 
> +static const struct device_type ethernet_type = {
> + .name = "ethernet",
> +};
> +
> /**
> *alloc_netdev_mqs - allocate network device
> *@sizeof_priv:   size of private data to allocate space for
> @@ -6305,6 +6309,8 @@ struct net_device *alloc_netdev_mqs(int 
> sizeof_priv, const char *name,
> goto free_all;
> #endif
> 
> + SET_NETDEV_DEVTYPE(dev, ðernet_type);
> +
> strcpy(dev->name, name);
> dev->group = INIT_NETDEV_GROUP;
> if (!dev->ethtool_ops)
 
 this means that every single netdev is defaulting to Ethernet. This 
 includes also the fake ones like IrDA or raw IP ones like PPP or TUN 
 devices. I do not think that is something we really want here.
>>> 
>>> Hm, so then the assumption I first worked under (DEVTYPE==(null) means
>>> ethernet) is not really correct. Doesn't this give you problems in
>>> ConnMan at the moment?
>> 
>> that is why I mentioned ARPHRD_* earlier. It is important to check that 
>> value as well. You will need both at some point.
> 
> Hm, I thought that was to fix false negatives. I.e., that some devices
> with, say, DEVTYPE=bluetooth should in fact be treated as regular
> ethernet?

DEVTYPE=bluetooth and DEVTYPE=wlan are not false negatives. They should be like 
that. That is something current userspace relies on. We can not break this.

However you will see a DEVTYPE=bluetooth as Ethernet emulation and another as 
raw IP (6loWPAN). You want to differentiate these for the default address 
assignment you pick. For example 6loWPAN will just rely on IPv6 route 
solicitation.

Same applies to DEVTYPE=wwan btw. Some Qualcomm cards can be exposed as 
Ethernet or they do a raw IP only header in their netdev. DEVTYPE is just to 
figure out the underlying technology of your network device. Then the ARPHRD_ 
is to get an idea of the actual encapsulation that is used.

Regards

Marcel

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

[systemd-devel] [RFC][PATCH v2] net: set default DEVTYPE for all ethernet based devices

[Tom Gundersen]

In systemd's networkd and udevd, we would like to give the administrator a
simple way to filter net devices by their DEVTYPE [0][1]. Other software
such as ConnMan and NetworkManager uses a similar filtering already.

Currently, plain ethernet devices have DEVTYPE=(null). This patch sets the
devtype to "ethernet" instead. This avoids the need for special-casing the
DEVTYPE=(null) case in userspace, and also avoids false positives, as there
are several other types of netdevs that also have DEVTYPE=(null).

Notice that this is done, as suggested by Marcel, in alloc_etherdev_mqs(),
and as best I can tell it will not give any false positives. I considered
doing it in ether_setup() instead as that seemed more intuitive, but that
would give a lot of false positives indeed.

[0]: 

[1]: 

Signed-off-by: Tom Gundersen 
Cc: Marcel Holtmann 
Cc: Greg KH 
---
 net/ethernet/eth.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
index 8f032ba..b76dc17 100644
--- a/net/ethernet/eth.c
+++ b/net/ethernet/eth.c
@@ -369,6 +369,10 @@ void ether_setup(struct net_device *dev)
 }
 EXPORT_SYMBOL(ether_setup);
 
+static const struct device_type eth_type = {
+   .name = "ethernet",
+};
+
 /**
  * alloc_etherdev_mqs - Allocates and sets up an Ethernet device
  * @sizeof_priv: Size of additional driver-private structure to be allocated
@@ -387,7 +391,13 @@ EXPORT_SYMBOL(ether_setup);
 struct net_device *alloc_etherdev_mqs(int sizeof_priv, unsigned int txqs,
  unsigned int rxqs)
 {
-   return alloc_netdev_mqs(sizeof_priv, "eth%d", ether_setup, txqs, rxqs);
+   struct net_device* dev;
+
+   dev = alloc_netdev_mqs(sizeof_priv, "eth%d", ether_setup, txqs, rxqs);
+   if (dev)
+   dev->dev.type = ð_type;
+
+   return dev;
 }
 EXPORT_SYMBOL(alloc_etherdev_mqs);
 
-- 
1.8.5.2

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

Re: [systemd-devel] [RFC][PATCH] net: set default DEVTYPE for all ethernet based devices

[Tom Gundersen]

On Sun, Jan 5, 2014 at 3:38 AM, Marcel Holtmann  wrote:
> Hi Tom,
>
 In systemd's networkd and udevd, we would like to give the administrator a
 simple way to filter ethernet devices by their DEVTYPE. In order to avoid
 having a special treatment of the case where DEVTYPE=(null), initialize it 
 to
 a default value, "ethernet", in the kernel.

 Signed-off-by: Tom Gundersen 
 Cc: Marcel Holtmann 
 Cc: Greg KH 
 ---

 Hi Greg and Marcel,

 This patch seems to do the right thing for me. Any comments before I send 
 it
 off to LKML?

 I suppose it may make sense to hide this behind a kernel option in case we 
 are
 worried about breaking existing users (but if ConnMan is adapted, I don't 
 know
 of any other issues, NetworkManager is not affected at least).

 Cheers,

 Tom

 net/core/dev.c | 6 ++
 1 file changed, 6 insertions(+)

 diff --git a/net/core/dev.c b/net/core/dev.c
 index ba3b7ea..62881e0 100644
 --- a/net/core/dev.c
 +++ b/net/core/dev.c
 @@ -6215,6 +6215,10 @@ void netdev_freemem(struct net_device *dev)
  kfree(addr);
 }

 +static const struct device_type ethernet_type = {
 + .name = "ethernet",
 +};
 +
 /**
 *alloc_netdev_mqs - allocate network device
 *@sizeof_priv:   size of private data to allocate space for
 @@ -6305,6 +6309,8 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, 
 const char *name,
  goto free_all;
 #endif

 + SET_NETDEV_DEVTYPE(dev, ðernet_type);
 +
  strcpy(dev->name, name);
  dev->group = INIT_NETDEV_GROUP;
  if (!dev->ethtool_ops)
>>>
>>> this means that every single netdev is defaulting to Ethernet. This 
>>> includes also the fake ones like IrDA or raw IP ones like PPP or TUN 
>>> devices. I do not think that is something we really want here.
>>
>> Hm, so then the assumption I first worked under (DEVTYPE==(null) means
>> ethernet) is not really correct. Doesn't this give you problems in
>> ConnMan at the moment?
>
> that is why I mentioned ARPHRD_* earlier. It is important to check that value 
> as well. You will need both at some point.

Hm, I thought that was to fix false negatives. I.e., that some devices
with, say, DEVTYPE=bluetooth should in fact be treated as regular
ethernet?

>> What do you think would be the correct behavior? To use this patch,
>> but also patch up IrDA, PPP, TUN and whatever else to set real
>> DEVTYPE's; or to leave them as essentially 'unknown', and go through
>> the ethernet drivers and set them to DEVTYPE='ethernet' one-by-one?
>
> The kernel has an alloc_etherdev_mqs() that might be a better starting point. 
> However even then, you might still need to fix some false positives.

Thanks for the hint. I'll take a look at starting from there, and then
rather hunt down any false positives we might produce.

Cheers,

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

Re: [systemd-devel] [PATCH] Fix format string mismatch introduced in ab9001a1

[Zbigniew Jędrzejewski-Szmek]

On Sun, Jan 05, 2014 at 05:06:50PM +0100, m...@zarb.org wrote:
> -asprintf(&b->address, UNIX_USER_BUS_FMT, (unsigned 
> long) getuid());
> +asprintf(&b->address, UNIX_USER_BUS_FMT, ee);
Ooops, thanks, applied.

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

[systemd-devel] [PATCH] Fix format string mismatch introduced in ab9001a1

[misc]

From: Michael Scherer 

src/libsystemd-bus/sd-bus.c: In function 'sd_bus_open_user':
src/libsystemd-bus/sd-bus.c:1104:25: warning: format '%s' expects argument of 
type 'char *', but argument 3 has type 'long unsigned int' [-Wformat=]
 asprintf(&b->address, UNIX_USER_BUS_FMT, (unsigned 
long) getuid());
---
 src/libsystemd-bus/sd-bus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/libsystemd-bus/sd-bus.c b/src/libsystemd-bus/sd-bus.c
index 61dc0e5..a4e4999 100644
--- a/src/libsystemd-bus/sd-bus.c
+++ b/src/libsystemd-bus/sd-bus.c
@@ -1101,7 +1101,7 @@ _public_ int sd_bus_open_user(sd_bus **ret) {
 #ifdef ENABLE_KDBUS
 asprintf(&b->address, KERNEL_USER_BUS_FMT ";" 
UNIX_USER_BUS_FMT, (unsigned long) getuid(), ee);
 #else
-asprintf(&b->address, UNIX_USER_BUS_FMT, (unsigned 
long) getuid());
+asprintf(&b->address, UNIX_USER_BUS_FMT, ee);
 #endif
 } else {
 #ifdef ENABLE_KDBUS
-- 
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 3/4] bus: driverd: don't attempt to remove from empty list

[Kay Sievers]

On Sun, Jan 5, 2014 at 11:37 AM, Kay Sievers  wrote:
> On Sat, Dec 28, 2013 at 8:54 AM, Marc-Antoine Perennou
>  wrote:
>> ---
>>  src/bus-driverd/bus-driverd.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> Applied.

And reverted it again, we need to find out what's really going wrong
here, not avoid the symptoms.

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

Re: [systemd-devel] [PATCH] journal: Add missing byte order conversions

[Zbigniew Jędrzejewski-Szmek]

On Tue, Dec 31, 2013 at 02:37:32PM -0600, George McCollister wrote:
> Convert entry_array.items[0] to host byte order prior to passing it to
> chain_cache_put().
Applied.

> Signed-off-by: George McCollister 
No need for this.

> --- a/src/journal/journal-file.c
> +++ b/src/journal/journal-file.c
> @@ -1452,7 +1452,7 @@ static int generic_array_get(
>  
>  found:
>  /* Let's cache this item for the next invocation */
> -chain_cache_put(f->chain_cache, ci, first, a, 
> o->entry_array.items[0], t, i);
> +chain_cache_put(f->chain_cache, ci, first, a, 
> le64toh(o->entry_array.items[0]), t, i);
I applied the same treatment to journal-verify.c. Can you check how
journalctl --verify behaves for you?

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

Re: [systemd-devel] [PATCH 5/5] strv: multiple cleanups

[Zbigniew Jędrzejewski-Szmek]

On Sun, Jan 05, 2014 at 12:52:31PM +0100, Peeters Simon wrote:
> 2014/1/5 Zbigniew Jędrzejewski-Szmek :
> > Looks great, except for one issue:
> >
> > On Sat, Jan 04, 2014 at 02:35:27AM +0100, Simon Peeters wrote:
> >> @@ -1865,14 +1863,11 @@ finish:
> >>  watchdog_close(false);
> >>
> >>  /* Tell the binary how often to ping */
> >> -snprintf(e, sizeof(e), "WATCHDOG_USEC=%llu", 
> >> (unsigned long long) arg_shutdown_watchdog);
> >> -char_array_0(e);
> >> +asprintf(&e, "WATCHDOG_USEC=%llu", (unsigned long 
> >> long) arg_shutdown_watchdog);
> >>
> >> -env_block = strv_append(environ, e);
> >> -} else {
> >> -env_block = strv_copy(environ);
> >> +strv_push(&env_block, e);
> > Should there be oom handling here?
> 
> there wasn't any in place, since this is shutdown code, I think we
> just need to avoid segfaulting on oom.
> but indeed, asprintf() doesn't set e to null on oom, so that should be:
> if (asprintf(&e, "WATCHDOG_USEC="USEC_FMT, arg_shutdown_watchdog) < 0)
>   e = NULL;
Right. I added the inverse of this (if (asprintf() > 0) strv_push()).

Applied.

Zbyszek

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

Re: [systemd-devel] [PATCH 3/5] shared: util.c: unify split and split_quoted

[Zbigniew Jędrzejewski-Szmek]

Applied.

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

Re: [systemd-devel] Create a new logind session from a systemd --user unit

[Tom Gundersen]

Hi Colin,

I realise this thread may be out-of-date by now, so please excuse me
if I'm commenting on something which has later changed.

On Sun, Aug 4, 2013 at 4:46 PM, Colin Walters  wrote:
> On Tue, 2013-07-30 at 01:02 +0200, Lennart Poettering wrote:
>
>> We are working on this bit by bit. If you want this to go faster, then
>> please work with us, and write patches for libX11 and D-Bus.
>
> Ok, some hacking on the plane on the way to GUADEC got me really far on
> this; then we had a quick face to face to work through some conceptual
> issues.
>
> In the new user@ model, it's very important to note there is only one
> "stub" process per session (at least graphical ones - whether we change
> getty is a whole other topic).  So for graphical, everything is launched
> from user@.service.  The side effect of this is twofold:
>
> 1) Pretty much all the user processes are no longer inside a session
>at all.
> 2) It is now much harder to log in multiple times graphically; this
>is kind of a crazy thing to do, but it's still *possible*.  To do
>so, you wire up your userspace code to set
>the pile of usual environment variables to override (DISPLAY,
>DBUS_SESSION_BUS_ADDRESS, etc.)  I can say though at least for
>GNOME we haven't supported this for years, so it's really not
>a change.
>
> For for adapting to 1), we agreed on adding the concept of a "primary"
> session, which is basically the first non-tty login.  I've added a small
> API to systemd to look this up, and the patched GNOME to use it
> (although we partially still respect XDG_SESSION_ID).

Hm, I'm not really happy with the notion of a "primary" session,
particularly as it relies on distinguishing between graphical and
non-graphical sessions (where does the line go between a traditional
VT, systemd-consoled, kmscon, wayland, X,...). And also relying on
something being the "first" sounds like it may become confusing for
the user.

Have you considered taking things one step further, and simply force
at most one logind session per user? This would mean that logging in
as the same user the second time means the new login will join the
already existing session. We then get:

 * If at most one of the logins is associated with a seat (and the
rest are remote), things will work just as now.

 * If two logins (of the same user) are associated with different
seats, this is not really supported at the moment, so we could ban it,
or we could simply allow it and temporarily merge the two seats and
let the session controller (window manager) deal with how that should
be presented to the user. This would probably mean that we want
logind's interfaces for getting hardware devices should be per-user,
and not per-session.

 * If two logins (of the same user) are both associated with the same
seat, we'll have to allow switching between the two apps being the
session controller within the same session (this is a common use-case
for when you login once with consoled and once with mutter, or
something like that).

Cheers,

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

Re: [systemd-devel] [PATCH 5/5] strv: multiple cleanups

[Peeters Simon]

2014/1/5 Zbigniew Jędrzejewski-Szmek :
> Looks great, except for one issue:
>
> On Sat, Jan 04, 2014 at 02:35:27AM +0100, Simon Peeters wrote:
>> @@ -1865,14 +1863,11 @@ finish:
>>  watchdog_close(false);
>>
>>  /* Tell the binary how often to ping */
>> -snprintf(e, sizeof(e), "WATCHDOG_USEC=%llu", 
>> (unsigned long long) arg_shutdown_watchdog);
>> -char_array_0(e);
>> +asprintf(&e, "WATCHDOG_USEC=%llu", (unsigned long 
>> long) arg_shutdown_watchdog);
>>
>> -env_block = strv_append(environ, e);
>> -} else {
>> -env_block = strv_copy(environ);
>> +strv_push(&env_block, e);
> Should there be oom handling here?

there wasn't any in place, since this is shutdown code, I think we
just need to avoid segfaulting on oom.
but indeed, asprintf() doesn't set e to null on oom, so that should be:
if (asprintf(&e, "WATCHDOG_USEC="USEC_FMT, arg_shutdown_watchdog) < 0)
  e = NULL;

and then it is equivalently oom safe as before, which means in worst
case passing a NULL env to the shutdown binary.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel