date:20140723

[systemd-devel] [PATCH 7/7] connection: fix user quota accounting corruption

2014-07-23 Thread Djalal Harouni

First use kzalloc to allocate the users array, so we do not reference
unintialized values.

And free the old conn->msg_users array not the newly allocated 'users'
one.

Patch tested, and users will hit the KDBUS_CONN_MAX_MSGS_PER_USER limit
and fail with -ENOBUFS

Signed-off-by: Djalal Harouni 
---
 connection.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/connection.c b/connection.c
index c432286..a2ed645 100644
--- a/connection.c
+++ b/connection.c
@@ -634,13 +634,13 @@ static int kdbus_conn_queue_user_quota(struct kdbus_conn 
*conn,
unsigned int i;
 
i = 8 + KDBUS_ALIGN8(user);
-   users = kmalloc(sizeof(unsigned int) * i, GFP_KERNEL);
+   users = kzalloc(sizeof(unsigned int) * i, GFP_KERNEL);
if (!users)
return -ENOMEM;
 
memcpy(users, conn->msg_users,
   sizeof(unsigned int) * conn->msg_users_max);
-   kfree(users);
+   kfree(conn->msg_users);
conn->msg_users = users;
conn->msg_users_max = i;
}
-- 
1.9.3

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

[systemd-devel] [PATCH 6/7] bus: call __kdbus_domain_user_account() and avoid an extra domain lock

2014-07-23 Thread Djalal Harouni

kdbus_bus_new() worst case will take the domain lock 3 times:

kdbus_bus_new()
 => kdbus_domain_user_find_or_new(): will take it 2 times
+
kdbus_bus_new(): will take it an extra time to account the user and
link the bus into the domain bus_list.

We can reduce the worst case to take the domain lock 2 times by
replacing the kdbus_domain_user_find_or_new() with

kdbus_domain_user_find(): take the lock 1 time
kdbus_domain_user_new()
+
kdbus_bus_new(): take the lock 1 time and use the unlocked version
__kdbus_domain_user_account() to account the user.

Signed-off-by: Djalal Harouni 
---
 bus.c| 21 +
 domain.h | 12 
 2 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/bus.c b/bus.c
index c329bef..77adf25 100644
--- a/bus.c
+++ b/bus.c
@@ -219,6 +219,7 @@ int kdbus_bus_new(struct kdbus_domain *domain,
struct kdbus_bus *b;
char prefix[16];
int ret;
+   bool new_user = false;
 
BUG_ON(*bus);
 
@@ -269,10 +270,15 @@ int kdbus_bus_new(struct kdbus_domain *domain,
if (ret < 0)
goto exit_free_reg;
 
-   /* account the bus against the user */
-   b->user = kdbus_domain_user_find_or_new(domain, uid);
-   if (IS_ERR(b->user)) {
-   ret = PTR_ERR(b->user);
+   /* account the bus against the user or create a new one */
+   b->user = kdbus_domain_user_find(domain, uid);
+   if (!b->user) {
+   b->user = kdbus_domain_user_new(domain, uid);
+   new_user = true;
+   }
+
+   if (!b->user) {
+   ret = -ENOMEM;
goto exit_ep_unref;
}
 
@@ -283,6 +289,13 @@ int kdbus_bus_new(struct kdbus_domain *domain,
goto exit_unref_user_unlock;
}
 
+   /* New user: account the bus against the user */
+   if (new_user) {
+   ret = __kdbus_domain_user_account(domain, b->user);
+   if (ret < 0)
+   goto exit_unref_user_unlock;
+   }
+
if (!capable(CAP_IPC_OWNER) &&
atomic_inc_return(&b->user->buses) > KDBUS_USER_MAX_BUSES) {
atomic_dec(&b->user->buses);
diff --git a/domain.h b/domain.h
index 9c477db..b17e023 100644
--- a/domain.h
+++ b/domain.h
@@ -99,6 +99,18 @@ int kdbus_domain_new(struct kdbus_domain *parent, const char 
*name,
 int kdbus_domain_make_user(struct kdbus_cmd_make *cmd, char **name);
 struct kdbus_domain *kdbus_domain_find_by_major(unsigned int major);
 
+struct kdbus_domain_user *
+kdbus_domain_user_new(struct kdbus_domain *domain, kuid_t uid);
+
+struct kdbus_domain_user *
+kdbus_domain_user_find(struct kdbus_domain *domain, kuid_t uid);
+
+int __kdbus_domain_user_account(struct kdbus_domain *domain,
+   struct kdbus_domain_user *user);
+
+int kdbus_domain_user_account(struct kdbus_domain *domain,
+ struct kdbus_domain_user *user);
+
 struct kdbus_domain_user
 *kdbus_domain_user_find_or_new(struct kdbus_domain *domain, kuid_t uid);
 struct kdbus_domain_user *kdbus_domain_user_ref(struct kdbus_domain_user *u);
-- 
1.9.3

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

[systemd-devel] [PATCH 5/7] kdbus: improve user domain accounting

2014-07-23 Thread Djalal Harouni

Currently kdbus_domain_user_find_or_new() is used to find a user domain
or create a new one and link it into the domain.

kdbus_domain_user_find_or_new() may fail due to memory allocation
errors or if the domain was shutdown, but since callers will receive
only a NULL pointer on failure, they assume -ENOMEM and ignore
-ESHUTDOWN.

To fix this we make kdbus_domain_user_find_or_new() return an ERR_PTR()
on failure, and we update all callers.

The patch also simplifies kdbus_domain_user_find_or_new() by using the
previously added helpers.

Another point is that by using kdbus_domain_user_account() inside
kdbus_domain_user_find_or_new() we make user index start from 1 instead
of 0, this allows:

Other parts of the code to directly call kdbus_domain_user_account() and
to be sure that user was really accounted and its index is 1 instead of
0.

0 is the default value given to user->idr after user = kzalloc(), if we
want other parts of the code to direclty account users, we must
differenciate the two values:

1) 0 as a valid user index
   user->idr  = idr_alloc(&domain->user_idr, user, 0, 0, GFP_KERNEL)

And

2) 0 after kzalloc()
   struct kdbus_domain_user *user = kzalloc()

So convert the former to 1, and adapt other parts of the code to treat 1
as the starting id of user indexes.

Signed-off-by: Djalal Harouni 
---
 bus.c|  4 ++--
 connection.c | 19 +++
 domain.c | 76 +---
 handle.c |  4 ++--
 4 files changed, 40 insertions(+), 63 deletions(-)

diff --git a/bus.c b/bus.c
index d09e3c6..c329bef 100644
--- a/bus.c
+++ b/bus.c
@@ -271,8 +271,8 @@ int kdbus_bus_new(struct kdbus_domain *domain,
 
/* account the bus against the user */
b->user = kdbus_domain_user_find_or_new(domain, uid);
-   if (!b->user) {
-   ret = -ENOMEM;
+   if (IS_ERR(b->user)) {
+   ret = PTR_ERR(b->user);
goto exit_ep_unref;
}
 
diff --git a/connection.c b/connection.c
index 1658a92..c432286 100644
--- a/connection.c
+++ b/connection.c
@@ -60,7 +60,7 @@ struct kdbus_conn_reply;
  * @dst_name_id:   The sequence number of the name this message is
  * addressed to, 0 for messages sent to an ID
  * @reply: The reply block if a reply to this message is expected.
- * @user:  Index in per-user message counter, -1 for unused
+ * @user:  Index in per-user message counter, 0 for unused
  */
 struct kdbus_conn_queue {
struct list_head entry;
@@ -78,7 +78,7 @@ struct kdbus_conn_queue {
u64 cookie;
u64 dst_name_id;
struct kdbus_conn_reply *reply;
-   int user;
+   unsigned int user;
 };
 
 /**
@@ -397,10 +397,10 @@ static void kdbus_conn_queue_remove(struct kdbus_conn 
*conn,
conn->msg_count--;
 
/* user quota */
-   if (queue->user >= 0) {
+   if (queue->user > 0) {
BUG_ON(conn->msg_users[queue->user] == 0);
conn->msg_users[queue->user]--;
-   queue->user = -1;
+   queue->user = 0;
}
 
/* the queue is empty, remove the user quota accounting */
@@ -471,8 +471,6 @@ static int kdbus_conn_queue_alloc(struct kdbus_conn *conn,
if (!queue)
return -ENOMEM;
 
-   queue->user = -1;
-
/* copy message properties we need for the queue management */
queue->src_id = kmsg->msg.src_id;
queue->cookie = kmsg->msg.cookie;
@@ -2092,12 +2090,13 @@ int kdbus_conn_new(struct kdbus_ep *ep,
 */
if (ep->user)
conn->user = kdbus_domain_user_ref(ep->user);
-   else
+   else {
conn->user = kdbus_domain_user_find_or_new(ep->bus->domain,
   current_fsuid());
-   if (!conn->user) {
-   ret = -ENOMEM;
-   goto exit_free_meta;
+   if (IS_ERR(conn->user)) {
+   ret = PTR_ERR(conn->user);
+   goto exit_free_meta;
+   }
}
 
/* lock order: domain -> bus -> ep -> names -> conn */
diff --git a/domain.c b/domain.c
index a5abb2d..81a9667 100644
--- a/domain.c
+++ b/domain.c
@@ -3,6 +3,7 @@
  * Copyright (C) 2013 Greg Kroah-Hartman 
  * Copyright (C) 2013 Daniel Mack 
  * Copyright (C) 2013 Linux Foundation
+ * Copyright (C) 2014 Djalal Harouni
  *
  * kdbus 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
@@ -540,64 +541,38 @@ int kdbus_domain_user_account(struct kdbus_domain *domain,
  * user like a custom endpoint
  *
  * Return: a kdbus_domain_user, either freshly allocated or with the reference
- * counter increased. In case of memory allocation failure, NULL is returned.
+ * counter increased. On failure, an ERR_PTR() that contains either -ENOMEM or
+ * -ESHUTDOWN is re

[systemd-devel] [PATCH 4/7] domain: add kdbus_domain_user_find()

2014-07-23 Thread Djalal Harouni

Add kdbus_domain_user_find() to look up domain users

Signed-off-by: Djalal Harouni 
---
 domain.c | 32 
 1 file changed, 32 insertions(+)

diff --git a/domain.c b/domain.c
index 18dc2a7..a5abb2d 100644
--- a/domain.c
+++ b/domain.c
@@ -446,6 +446,38 @@ kdbus_domain_user_new(struct kdbus_domain *domain, kuid_t 
uid)
 }
 
 /**
+ * kdbus_domain_user_find() - find a domain user with a given uid
+ * @domain The domain of the user
+ * @uidThe uid of the user; INVALID_UID for an
+ * anonymous user like custom endpoint
+ *
+ * Return: a kdbus_domain_user with the reference counter increased.
+ * If the domain user can't be found, NULL is returned.
+ */
+struct kdbus_domain_user *
+kdbus_domain_user_find(struct kdbus_domain *domain, kuid_t uid)
+{
+   struct kdbus_domain_user *u;
+   struct kdbus_domain_user *user = NULL;
+
+   if (!uid_valid(uid))
+   return user;
+
+   mutex_lock(&domain->lock);
+   hash_for_each_possible(domain->user_hash, u,
+  hentry, __kuid_val(uid)) {
+   if (!uid_eq(u->uid, uid))
+   continue;
+
+   user = kdbus_domain_user_ref(u);
+   break;
+   }
+   mutex_unlock(&domain->lock);
+
+   return user;
+}
+
+/**
  * __kdbus_domain_user_account() - account a kdbus_domain_user object
  *into the specified domain
  * @domain:The domain of the user
-- 
1.9.3

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

[systemd-devel] [PATCH 3/7] domain: add kdbus_domain_user_new()

2014-07-23 Thread Djalal Harouni

Add kdbus_domain_user_new() to allocate kdbus_domain_user objects.

Signed-off-by: Djalal Harouni 
---
 domain.c | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/domain.c b/domain.c
index 86fde55..18dc2a7 100644
--- a/domain.c
+++ b/domain.c
@@ -419,6 +419,33 @@ int kdbus_domain_make_user(struct kdbus_cmd_make *cmd, 
char **name)
 }
 
 /**
+ * kdbus_domain_user_new() - create a new kdbus_domain_user object
+ * @domain The domain of the user
+ * @uidThe uid of the user; INVALID_UID for an
+ * anonymous user like custom endpoint
+ *
+ * Return: a kdbus_domain_user freshly allocated. In case of memory
+ * allocation failure, NULL is returned.
+ */
+struct kdbus_domain_user *
+kdbus_domain_user_new(struct kdbus_domain *domain, kuid_t uid)
+{
+   struct kdbus_domain_user *u;
+
+   u = kzalloc(sizeof(*u), GFP_KERNEL);
+   if (!u)
+   return NULL;
+
+   kref_init(&u->kref);
+   u->domain = kdbus_domain_ref(domain);
+   u->uid = uid;
+   atomic_set(&u->buses, 0);
+   atomic_set(&u->connections, 0);
+
+   return u;
+}
+
+/**
  * __kdbus_domain_user_account() - account a kdbus_domain_user object
  *into the specified domain
  * @domain:The domain of the user
-- 
1.9.3

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

[systemd-devel] [PATCH 2/7] domain: add the lock protected version of user accounting

2014-07-23 Thread Djalal Harouni

Add the lock protected version of __kdbus_domain_user_account(). It
will check if the domain is still active before linking users.

Signed-off-by: Djalal Harouni 
---
 domain.c | 24 
 1 file changed, 24 insertions(+)

diff --git a/domain.c b/domain.c
index a321f31..86fde55 100644
--- a/domain.c
+++ b/domain.c
@@ -451,6 +451,30 @@ int __kdbus_domain_user_account(struct kdbus_domain 
*domain,
 }
 
 /**
+ * kdbus_domain_user_account() - account a kdbus_domain_user object
+ *  into the specified domain
+ * @domain:The domain of the user
+ * @user   The kdbus_domain_user object of the user
+ *
+ * Returns 0 on success. In case of memory allocation failure return
+ * -ENOMEM, or -ESHUTDOWN if the domain was disconnected.
+ */
+int kdbus_domain_user_account(struct kdbus_domain *domain,
+ struct kdbus_domain_user *user)
+{
+   int ret = -ESHUTDOWN;
+
+   mutex_lock(&domain->lock);
+
+   if (!domain->disconnected)
+   ret = __kdbus_domain_user_account(domain, user);
+
+   mutex_unlock(&domain->lock);
+
+   return ret;
+}
+
+/**
  * kdbus_domain_user_find_or_new() - get a kdbus_domain_user object in a domain
  * @domain:The domain
  * @uid:   The uid of the user; INVALID_UID for an anonymous
-- 
1.9.3

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

[systemd-devel] [PATCH 1/7] domain: add __kdbus_domain_user_account() to account and link users

2014-07-23 Thread Djalal Harouni

Add __kdbus_domain_user_account() to account and link users into a
domain.

Signed-off-by: Djalal Harouni 
---
 domain.c | 32 
 1 file changed, 32 insertions(+)

diff --git a/domain.c b/domain.c
index c4912fa..a321f31 100644
--- a/domain.c
+++ b/domain.c
@@ -419,6 +419,38 @@ int kdbus_domain_make_user(struct kdbus_cmd_make *cmd, 
char **name)
 }
 
 /**
+ * __kdbus_domain_user_account() - account a kdbus_domain_user object
+ *into the specified domain
+ * @domain:The domain of the user
+ * @user   The kdbus_domain_user object of the user
+ *
+ * Returns 0 on success, -ENOMEM on failure.
+ *
+ * Caller must have the domain lock held and must ensure that the
+ * domain was not disconnected.
+ */
+int __kdbus_domain_user_account(struct kdbus_domain *domain,
+   struct kdbus_domain_user *user)
+{
+   int ret;
+
+   /*
+* Allocate the smallest possible index for this user; used
+* in arrays for accounting user quota in receiver queues.
+*/
+   ret = idr_alloc(&domain->user_idr, user, 1, 0, GFP_KERNEL);
+   if (ret < 0)
+   return ret;
+
+   user->idr = ret;
+
+   /* UID hash map */
+   hash_add(domain->user_hash, &user->hentry, __kuid_val(user->uid));
+
+   return 0;
+}
+
+/**
  * kdbus_domain_user_find_or_new() - get a kdbus_domain_user object in a domain
  * @domain:The domain
  * @uid:   The uid of the user; INVALID_UID for an anonymous
-- 
1.9.3

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

[systemd-devel] [PATCH 0/7] kdbus: improve user domain accounting

2014-07-23 Thread Djalal Harouni

Hi,

This series improves user domain accounting and fixes some bugs.

On top of the "kdbus: allow multiple policies" series:
http://lists.freedesktop.org/archives/systemd-devel/2014-July/021514.html


Patches 1, 2, 3 and 4 are preparation patches to improve the code.

Patch 5 fixes kdbus_domain_user_find_or_new(), callers assume that
kdbus_domain_user_find_or_new() will fail only with -ENOMEM, but it
may fail with -ESHUTDOWN, so adapt the code to return the appropriate
errors and fix all callers.

Patch 6 makes use of the new helpers in order to reduce the number
of domain locks taken in kdbus_bus_new(). The domain is the upper layer,
reducing the locks should reduce some overheads.

In order to reduce the number of locks in this patch 6, we make the
domain->user_idr start from 1, so if a user->idr == 0 we assume that
it was not accounted. This logic was used in patch 5 which contains
the detailed explanation.

Patch 7 fixes a user quota accounting corruption.


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

Re: [systemd-devel] sysusers and login.defs checks

2014-07-23 Thread Colin Guthrie

Lennart Poettering wrote on 23/07/14 18:31:
> On Tue, 22.07.14 18:35, Colin Guthrie (gm...@colin.guthr.ie) wrote:
> 
>>
>> 'Twas brillig, and Lennart Poettering at 22/07/14 12:10 did gyre and gimble:
> I guess it's OK to do this kind of user lookup stuff from the journal
> code (i.e. server_fix_perms())?
>>> Hmm, yuck. Actually it is really difficult
>>> ...
>>> Bummer, not sure if we can save this idea...
>>
>> Yeah, I did wonder about it when you suggested it!
> 
> Talked to Kay about this a bit more. Here's an idea:
> 
> There are basically three areas where the system vs. regular user UID
> boundary matters:
> 
> a) in journald for splitting up journals for individual users
> b) in the coredump hook, for similar purposes
> c) in sysusers when creating new system users
> 
> Solution for a): add a new configuration option to journald.conf for
> declaring the UID range to split up journals in. Usage like this:
>   
>   SplitUserRange=1000-65533
> 
> Solution for b): similar, but an option for coredump.conf
> 
> Solution for c): a new "r" directive or so for the sysusers snippets
> that declares ranges to allocate new system users from:
> 
>  r - 200-999
> 
> In all three cases, if the setting is not set, we default to the
> configure time boundary (1000) as before.
> 
> To make this generic, we'd actually allow people to configure multiple
> ranges freely:
> 
>SplitUserRange=1000-2000,1-6533
> 
> or for sysusers.d
> 
> r - 200-700
> r - 800-999
> 
> Now, this alone wouldn't provide compatibility with the dreaded
> login.defs file. For that we'd then employ a postinst script that reads
> the range from the file, and then automatically generates a sysuers.d
> drop-in or a patches journald.conf and coredump.conf should the range
> not match the default.
> 
> Does this make sense?

To be frank, I really don't think it does make much sense at all. I
mean, something which is currently configured in one place and then used
in tools such as shadow-utils when creating users (both with and without
-s) now has to be configured in three *additional* places. That hardly
seems like an improvement and sounds like a recipe for misconfiguration.
It just makes it more convoluted, where essentially the same data is
repeated in multiple places and then you have some crazy config file
patching system on top of that to distribute it to all those places.

If you were to suggest some new shared configuration file and we could
update shadow-utils and friends to use it then fine (seems kinda
pointless tho'), but I really don't get the logic here.

Ultimately, I guess really don't care much about it. For fresh installs
it's kinda moot, but that said, login.defs is fairly well known and I
really just don't grok the aversion to using it if it exists.

I'd rather we just refactored systemd to have a utility function
"is_system_uid()" or such like which just does the compiled in default
check as it does now (no functional change to how it works now, just
centralised) and then I can just hack in reading and honouring
login.defs into that utility function in a downstream patch. It would be
a couple lines of code and very easy to support downstream if you are so
against it. I'm not against a compiled in default (that's what
shadow-utils does too after all)

> As a side effect this would actually even allow us to be closer to
> FEdora's current bheaviour, since it reserves UIDs < 200 for static
> assignment, which we could then easily exclude from theis logic, too.

Yeah that's one advantage, but I don't really see why this is any better
than just *one* configuration value (e.g. SYS_UID_MIN) in one
configuration file (e.g. /etc/login.defs) which is already honoured by
shadow-utils and no doubt other tools too.

This seems a case were your suggesting a change for changes sake rather
than just saying fine, login.defs is as good a config file as any for
picking vaguely sane defaults. Perhaps I'm missing some background as to
why login.defs is such a horrible config file?

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] sysusers and login.defs checks

2014-07-23 Thread Simon McVittie

On 23/07/14 19:50, Zbigniew Jędrzejewski-Szmek wrote:
>> As a side effect this would actually even allow us to be closer to
>> FEdora's current bheaviour, since it reserves UIDs < 200 for static
>> assignment, which we could then easily exclude from theis logic, too.
> In practice this might not be useful, because even if all the 800 UIDs
> starting from 999 were used up, it would be better to encroach on the
> "static" range than to fail.

At least in Debian and its derivatives, the statically-allocated ranges
0-99 and 6-64999 are basically for two things: "well-known" users
like root/daemon/ftp/www-data, and packages that hard-code a numeric uid
at compile time. Yes, the latter is probably a design flaw in the
package that needs that uid[1]; but as long as such software exists,
installing it will result in it using that uid, whether it's already in
use by something else or not.

In this unlikely situation, it seems better to fail than to operate
insecurely; or if it's absolutely necessary to allocate a new dynamic
uid, taking one from the "real user" range seems less risky than taking
one from a statically-allocated range.

[1] arguably; iirc Apache suexec deliberately hard-codes www-data's uid
at compile time as a security measure, although that admittedly seems
more like security theatre than actual security, because if you can't
trust nsswitch then you've already lost

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

Re: [systemd-devel] sysusers and login.defs checks

2014-07-23 Thread Simon McVittie

On 23/07/14 18:31, Lennart Poettering wrote:
> As a side effect this would actually even allow us to be closer to
> FEdora's current bheaviour, since it reserves UIDs < 200 for static
> assignment, which we could then easily exclude from theis logic, too.

Debian's uid/gid ranges for comparison, if it helps:

I think Ubuntu inherits those too, they generally follow Debian policy
if they have no reason not to.

(0-99 system users statically allocated by Debian and normally
pre-created, 100-999 dynamically allocated system users, 1000-5
dynamically allocated real users, 6000-64999 statically allocated by
Debian but only created on demand, 65000-65533 reserved, 65534
statically allocated as nobody, 65535 reserved because it is or was once
(uid_t)(-1); changing the boundary between the dynamically allocated
ranges in login.defs is best-effort-supported for the benefit of people
whose site-wide real users start at 500 or whatever.)

S

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

Re: [systemd-devel] sysusers and login.defs checks

2014-07-23 Thread Zbigniew Jędrzejewski-Szmek

On Wed, Jul 23, 2014 at 07:31:31PM +0200, Lennart Poettering wrote:

[snip]

> Now, this alone wouldn't provide compatibility with the dreaded
> login.defs file. For that we'd then employ a postinst script that reads
> the range from the file, and then automatically generates a sysuers.d
> drop-in or a patches journald.conf and coredump.conf should the range
> not match the default.
> 
> Does this make sense?
Seems like a great big effort to avoid reading a simple configuration
file.

When people want to teach systemd to generate dynamic configuration
based on some other configuration, to pass commandline options and/or
environment variables to a daemon that is starting up, because the
daemon cannot do it on its own, we always tell them: teach the daemon
to do that on its own. We should apply the same principle here, and
not spread the configuration over n automatically generated files.

> As a side effect this would actually even allow us to be closer to
> FEdora's current bheaviour, since it reserves UIDs < 200 for static
> assignment, which we could then easily exclude from theis logic, too.
In practice this might not be useful, because even if all the 800 UIDs
starting from 999 were used up, it would be better to encroach on the
"static" range than to fail.

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

Re: [systemd-devel] Changing configurations with networkd

2014-07-23 Thread poma


On 23.07.2014 16:46, Marcel Holtmann wrote:

Hi Michael,


I think the lease should be remembered and reused in this case.


Hm, this sounds like a bug somewhere. When the new discover is sent
out it should send the same identifying information to the server, and
hence be given the same lease back again. Wireshark should tell you if
the fault is networkd's or the DHCP server.


I get the same address at first, but after several minutes the DHCP server
decides to offer a new address. I should note, that I have a 10 minute
lease time for debugging purposes, so that might make the problem more
prominent. I'll see if I can figure out what happens here.


look at your DHCP server and see what lease time it really hands out after 
reboot.

However this is between you and your DHCP server. If you configure a lease time 
of 10 minutes, then that is the only guaranteed time for a given IP address. 
There is no mandate that the server has to give you the same address after 10 
minutes when you ask again. It is valid to just get a different one. And that 
many home routers try to give you back the same one does not mean that they are 
required to do so.

The nice DHCP servers will remember your Ethernet address and/or identity 
information and give you back your old IP address. Either with the left over 
lease time or with a brand new lease time. There is really no need to store 
this information on disk. If the lease expired the information on disk are 
stale as well. And since our DHCP implementation is so fast, it makes really no 
difference.

It is safer start out with a brand new DHCP lease instead of having to deal 
with renewal during boot. At least that way you know the DHCP server is still 
there and you have a valid IP address. Just re-using a stored IP with a 
left-over lease is not safe anyway. You never know what changed in the network 
when you were off.

Regards

Marcel



Exactly, let him configures static leases, and that's it.

http://en.wikipedia.org/wiki/Dynamic_Host_Configuration_Protocol
   static allocation: The DHCP server allocates an IP address based on a 
preconfigured mapping to each client's MAC address.
   This feature is variously called static DHCP assignment by DD-WRT, 
fixed-address by the dhcpd documentation,
   address reservation by Netgear, DHCP reservation or static DHCP by Cisco and 
Linksys,
   and IP address reservation or MAC/IP address binding by various other router 
manufacturers.


networkd fan club


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

Re: [systemd-devel] sysusers and login.defs checks

2014-07-23 Thread Lennart Poettering

On Tue, 22.07.14 18:35, Colin Guthrie (gm...@colin.guthr.ie) wrote:

> 
> 'Twas brillig, and Lennart Poettering at 22/07/14 12:10 did gyre and gimble:
> >> > I guess it's OK to do this kind of user lookup stuff from the journal
> >> > code (i.e. server_fix_perms())?
> > Hmm, yuck. Actually it is really difficult
> > ...
> > Bummer, not sure if we can save this idea...
> 
> Yeah, I did wonder about it when you suggested it!

Talked to Kay about this a bit more. Here's an idea:

There are basically three areas where the system vs. regular user UID
boundary matters:

a) in journald for splitting up journals for individual users
b) in the coredump hook, for similar purposes
c) in sysusers when creating new system users

Solution for a): add a new configuration option to journald.conf for
declaring the UID range to split up journals in. Usage like this:

  SplitUserRange=1000-65533

Solution for b): similar, but an option for coredump.conf

Solution for c): a new "r" directive or so for the sysusers snippets
that declares ranges to allocate new system users from:

 r - 200-999

In all three cases, if the setting is not set, we default to the
configure time boundary (1000) as before.

To make this generic, we'd actually allow people to configure multiple
ranges freely:

   SplitUserRange=1000-2000,1-6533

or for sysusers.d

r - 200-700
r - 800-999

Now, this alone wouldn't provide compatibility with the dreaded
login.defs file. For that we'd then employ a postinst script that reads
the range from the file, and then automatically generates a sysuers.d
drop-in or a patches journald.conf and coredump.conf should the range
not match the default.

Does this make sense?

As a side effect this would actually even allow us to be closer to
FEdora's current bheaviour, since it reserves UIDs < 200 for static
assignment, which we could then easily exclude from theis logic, too.

Lennart

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

[systemd-devel] [PATCH 5/5] connection: allow policy holders to install multiple names

2014-07-23 Thread Djalal Harouni

Signed-off-by: Djalal Harouni 
---
 connection.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/connection.c b/connection.c
index 85ffa5a..1658a92 100644
--- a/connection.c
+++ b/connection.c
@@ -1905,7 +1905,11 @@ int kdbus_conn_new(struct kdbus_ep *ep,
if (!is_activator && !is_policy_holder)
return -EINVAL;
 
-   if (name)
+   /*
+* activators are allowed to install only
+* one name.
+*/
+   if (name && is_activator)
return -EINVAL;
 
if (!kdbus_item_validate_nul(item))
-- 
1.9.3

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

[systemd-devel] [PATCH 4/5] test: register multiple policies

2014-07-23 Thread Djalal Harouni

Update the policy test in order to register multiple policies

Signed-off-by: Djalal Harouni 
---
 test/test-kdbus-policy.c | 57 +---
 1 file changed, 40 insertions(+), 17 deletions(-)

diff --git a/test/test-kdbus-policy.c b/test/test-kdbus-policy.c
index de725e9..c12c3ad 100644
--- a/test/test-kdbus-policy.c
+++ b/test/test-kdbus-policy.c
@@ -27,8 +27,8 @@
 #include "kdbus-enum.h"
 
 #define MAX_CONN   64
-#define POLICY_NAME"foo.test.policy-test"
-
+#define POLICY_NAME"foo.test.policy-test"
+#define POLICY_NAME_DUMMY_A"foo.test.policy-dummy-a"
 
 /**
  * The purpose of these tests:
@@ -81,21 +81,15 @@ static int kdbus_register_activator(char *bus, const char 
*name,
return 0;
 }
 
-static int kdbus_register_policy_holder(char *bus, const char *name,
-   struct conn **conn)
+static int
+kdbus_register_policy_holder(char *bus,
+const struct kdbus_policy_entry *entries,
+size_t num_entries,
+struct conn **conn)
 {
struct conn *c;
-   struct kdbus_policy_access access[2];
-
-   access[0].type = KDBUS_POLICY_ACCESS_USER;
-   access[0].access = KDBUS_POLICY_OWN;
-   access[0].id = geteuid();
 
-   access[1].type = KDBUS_POLICY_ACCESS_WORLD;
-   access[1].access = KDBUS_POLICY_TALK;
-   access[1].id = geteuid();
-
-   c = kdbus_hello_registrar(bus, name, access, 2,
+   c = kdbus_hello_registrar(bus, entries, num_entries,
  KDBUS_HELLO_POLICY_HOLDER);
if (!c)
return -errno;
@@ -268,7 +262,34 @@ static int kdbus_check_policy(char *bus)
 {
int i;
int ret;
+   size_t num_entries;
struct conn *activator = NULL;
+   struct kdbus_policy_access policy_access[2] = {
+   {
+   .type = KDBUS_POLICY_ACCESS_USER,
+   .access = KDBUS_POLICY_OWN,
+   .id = geteuid(),
+   },
+   {
+   .type = KDBUS_POLICY_ACCESS_WORLD,
+   .access = KDBUS_POLICY_TALK,
+   .id = geteuid(),
+   },
+   };
+
+   struct kdbus_policy_entry policy_entries[] = {
+   {
+   .name = POLICY_NAME,
+   .num_access = 2,
+   .access = policy_access,
+   },
+   {
+   .name = POLICY_NAME_DUMMY_A,
+   .num_access = 0,
+   },
+   };
+
+   num_entries = sizeof(policy_entries)/sizeof(struct kdbus_policy_entry);
 
conn_db = calloc(MAX_CONN, sizeof(struct conn *));
if (!conn_db)
@@ -276,7 +297,8 @@ static int kdbus_check_policy(char *bus)
 
memset(conn_db, 0, MAX_CONN * sizeof(struct conn *));
 
-   ret = kdbus_register_policy_holder(bus, POLICY_NAME, &conn_db[0]);
+   ret = kdbus_register_policy_holder(bus, policy_entries,
+  num_entries, &conn_db[0]);
printf("-- TEST 1) register '%s' as policy holder ",
POLICY_NAME);
if (ret < 0) {
@@ -287,9 +309,10 @@ static int kdbus_check_policy(char *bus)
printf("OK\n");
 
/* Try to register the same name with an activator */
-   ret = kdbus_register_activator(bus, POLICY_NAME, &activator);
+   ret = kdbus_register_activator(bus, policy_entries[0].name,
+  &activator);
printf("-- TEST 2) register again '%s' as an activator ",
-   POLICY_NAME);
+   policy_entries[0].name);
if (ret == 0) {
printf("succeeded: TEST FAILED\n");
fprintf(stderr, "--- error was able to register twice '%s'.\n",
-- 
1.9.3

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

[systemd-devel] [PATCH 3/5] test: add infrastructure to allow multiple policies per connection

2014-07-23 Thread Djalal Harouni

A policy holder should be able to upload multiple policies, so add the
test infrastructure for it.

Convert kdbus_hello_registrar() to allow an array of
'struct kdbus_policy_entry' and add the following helpers:

kdbus_policy_entries_size() to calculate the KDBUS_ITEM policies size

kdbus_policy_make_item_name() to make a policy item composed of:
  KDBUS_ITEM_NAME + (num_access * KDBUS_ITEM_POLICY_ACCESS)

kdbus_policy_make_entries() a helper to construct multiple policies

Signed-off-by: Djalal Harouni 
---
 test/kdbus-util.c | 99 +++
 test/kdbus-util.h |  6 ++--
 2 files changed, 73 insertions(+), 32 deletions(-)

diff --git a/test/kdbus-util.c b/test/kdbus-util.c
index 6bb3bbf..1ce6050 100644
--- a/test/kdbus-util.c
+++ b/test/kdbus-util.c
@@ -111,22 +111,34 @@ struct conn *kdbus_hello(const char *path, uint64_t flags)
return __kdbus_hello(path, flags, NULL, 0);
 }
 
-struct conn *
-kdbus_hello_registrar(const char *path, const char *name,
- const struct kdbus_policy_access *access,
- size_t num_access, uint64_t flags)
+static size_t kdbus_policy_entries_size(const struct kdbus_policy_entry 
*entries,
+   size_t num_entries)
 {
-   struct kdbus_item *item, *items;
-   size_t i, size;
+   size_t i;
+   size_t size = 0;
 
-   size = KDBUS_ITEM_SIZE(strlen(name) + 1)
-   + num_access * KDBUS_ITEM_SIZE(sizeof(struct 
kdbus_policy_access));
+   for (i = 0; i < num_entries; i++) {
+   const struct kdbus_policy_entry *e = entries + i;
 
-   items = alloca(size);
+   size += KDBUS_ITEM_SIZE(strlen(e->name) + 1);
+   size += e->num_access * KDBUS_ITEM_SIZE(sizeof(struct 
kdbus_policy_access));
+   }
+
+   return size;
+}
+
+/* Make sure that the 'item' parameter points to a valid item. */
+static struct kdbus_item *
+kdbus_policy_make_item_name(const char *name,
+   const struct kdbus_policy_access *access,
+   size_t num_access,
+   struct kdbus_item *item)
+{
+   size_t i;
 
-   item = items;
-   item->size = KDBUS_ITEM_HEADER_SIZE + strlen(name) + 1;
item->type = KDBUS_ITEM_NAME;
+   item->size = KDBUS_ITEM_HEADER_SIZE + strlen(name) + 1;
+
strcpy(item->str, name);
item = KDBUS_ITEM_NEXT(item);
 
@@ -142,6 +154,44 @@ kdbus_hello_registrar(const char *path, const char *name,
item = KDBUS_ITEM_NEXT(item);
}
 
+   return item;
+}
+
+/* Make sure that the 'item' parameter points to a valid item. */
+static struct kdbus_item *
+kdbus_policy_make_entries(const struct kdbus_policy_entry *entries,
+ size_t num_entries,
+ struct kdbus_item *item)
+{
+   size_t i;
+
+   for (i = 0; i < num_entries; i++) {
+   const struct kdbus_policy_entry *e = &entries[i];
+
+   item = kdbus_policy_make_item_name(e->name, e->access,
+  e->num_access, item);
+   }
+
+   return item;
+}
+
+struct conn *
+kdbus_hello_registrar(const char *path,
+ const struct kdbus_policy_entry *entries,
+ size_t num_entries, uint64_t flags)
+{
+   size_t size;
+   struct kdbus_item *item, *items;
+
+   size = kdbus_policy_entries_size(entries, num_entries);
+   items = alloca(size);
+
+   memset(items, 0, size);
+
+   item = items;
+   item = kdbus_policy_make_entries(entries,
+num_entries, item);
+
return __kdbus_hello(path, flags, items, size);
 }
 
@@ -149,7 +199,13 @@ struct conn *kdbus_hello_activator(const char *path, const 
char *name,
   const struct kdbus_policy_access *access,
   size_t num_access)
 {
-   return kdbus_hello_registrar(path, name, access, num_access,
+   struct kdbus_policy_entry entry = {
+   .name = (char *)name,
+   .num_access = num_access,
+   .access = (struct kdbus_policy_access *)access,
+   };
+
+   return kdbus_hello_registrar(path, &entry, 1,
 KDBUS_HELLO_ACTIVATOR);
 }
 
@@ -613,7 +669,7 @@ int conn_update(struct conn *conn, const char *name,
 {
struct kdbus_cmd_update *update;
struct kdbus_item *item;
-   size_t i, size;
+   size_t size;
int ret;
 
size = sizeof(struct kdbus_cmd_update);
@@ -652,22 +708,7 @@ int conn_update(struct conn *conn, const char *name,
  KDBUS_ATTACH_CONN_NAME;
item = KDBUS_ITEM_NEXT(item);
 
-   item->type = KDBUS_ITEM_NAME;
-   item->size = KDBUS_ITEM_HEADER_SIZE + strlen(name) + 1;
-   strcpy(item->str, name);
-   item = KDBUS_ITEM_NEXT

[systemd-devel] [PATCH 2/5] test: add the struct kdbus_policy_entry to be used for tests

2014-07-23 Thread Djalal Harouni

Signed-off-by: Djalal Harouni 
---
 test/kdbus-util.h | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/test/kdbus-util.h b/test/kdbus-util.h
index 0fcfb72..39d7bb5 100644
--- a/test/kdbus-util.h
+++ b/test/kdbus-util.h
@@ -36,6 +36,12 @@ struct conn {
size_t size;
 };
 
+struct kdbus_policy_entry {
+   char *name;
+   size_t num_access;
+   struct kdbus_policy_access *access;
+};
+
 int name_list(struct conn *conn, uint64_t flags);
 int name_release(struct conn *conn, const char *name);
 int name_acquire(struct conn *conn, const char *name, uint64_t flags);
-- 
1.9.3

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

[systemd-devel] [PATCH 1/5] test: correctly set the 'ret' variable

2014-07-23 Thread Djalal Harouni

Signed-off-by: Djalal Harouni 
---
 test/test-kdbus-policy.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/test/test-kdbus-policy.c b/test/test-kdbus-policy.c
index 6099087..de725e9 100644
--- a/test/test-kdbus-policy.c
+++ b/test/test-kdbus-policy.c
@@ -200,7 +200,7 @@ static int kdbus_normal_test(const char *bus, const char 
*name,
 static int kdbus_fork_test(const char *bus, const char *name,
   struct conn **conn_db)
 {
-   int ret;
+   int ret = 0;
int status;
pid_t pid;
int test_done = 0;
@@ -241,6 +241,7 @@ child_fail:
 
ret = waitpid(pid, &status, 0);
if (ret < 0) {
+   ret = -errno;
fprintf(stderr, "error waitpid: %d (%m)\n", ret);
goto out;
}
-- 
1.9.3

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

[systemd-devel] [PATCH 0/5] kdbus: allow multiple policies

2014-07-23 Thread Djalal Harouni

This series adds the infrastructure to test and upload multiple
policies.

The last #5 patch allows to upload multiple policies per connection


The todo for the policy holders is:

* Should we set a maximum value for how many names/policies a policy
 holder is allowed to upload. This is needed since we do not want to
 consume all the entries (kdbus_policy_db->entries_hash).

* Update/test the kdbus_policy_set() to work with multiple names.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] sysusers and login.defs checks

2014-07-23 Thread Zbigniew Jędrzejewski-Szmek

On Wed, Jul 23, 2014 at 05:30:53PM +0200, Kay Sievers wrote:
> On Wed, Jul 23, 2014 at 5:17 PM, Zbigniew Jędrzejewski-Szmek
>  wrote:
> > On Wed, Jul 23, 2014 at 04:55:59PM +0200, Kay Sievers wrote:
> >> On Wed, Jul 23, 2014 at 4:28 PM, Zbigniew Jędrzejewski-Szmek
> >>  wrote:
> >>
> >> > Anyway, I think that /etc/login.defs support is made out to be something
> >> > much more complicated than it really is. IMHO we should:
> >> >
> >> > - read /etc/login.defs and fall back to the compiled in value
> >> > - use whatever result we get in coredump, journald, and sysusers
> >> >
> >> > It's not like the implementation would be hard, intrusive, or slow. It'd 
> >> > be
> >> > probably +3 lines in one or two places.
> >>
> >> It is not about the effort *how* to do it, it is *why*. And I still
> >> don't think we should have dynamic configuration options for this, it
> >> is all just a huge mess that needs to be limited to the bare minimum,
> >> it is just too broken as a concept to be supported that way.
> >>
> >> > If we come up with additional heuristics or rules to determine human
> >> > accounts, we can safely add them in a backwards compatible way.
> >>
> >> We cannot do any normal user queries from journald, so none of the
> >> metadata like the primary group is easily for a user is available.
> > I know.
> >
> >> Sysusers is, and probably always will be, limited to the classic
> >> passwd, group file. Maybe we can just read the files ourselves and
> >> find out that a certain uid is a normal user? Like:
> >>   - uid >= "1000" --> normal user
> >>   - lookup uid in passwd
> >>   - user not found --> normal user
> >>   - user < 1000 && group == "users" --> normal user
> >>   - everything else would be a system user
> > But please add to this (at the top)
> > - parse SYS_GID_MIN and SYS_GID_MAX from /etc/login.defs and if
> >   found and users falls within --> system user
> >
> > This is safe as soon as /etc is accessible and provides backwards
> > compatibillity.
> 
> Well, the point is to make the rules in this broken model simpler, not
> more complicated as they already are. :)
> 
> If we would read login.defs, we should probably not do any magic
> heuristics. And at the moment, I still think we should ignore
> login.defs.
If we find it, then certainly, it should override other considerations.

> > Also, I'd modify
> > - user < 1000 && group == "users" --> normal user
> > to
> > - group == "users" --> normal user
> > not to make things too complicated.
> >
> > I see another angry chicken and broken egg problem now:
> > - We want to get rid of /etc/login.defs, *but*
> > - we read /etc/login.defs at compilation time.
> > This means that we probably should stop looking at that file during
> > compilation time and stick to an internal default, possibly allowing
> > overriding with ./configure switch.
> 
> Maybe, yes. It was just to init the build sys with the current distro 
> defaults.
Right, but it makes login.defs even more entrenched.

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

Re: [systemd-devel] sysusers and login.defs checks

2014-07-23 Thread Kay Sievers

On Wed, Jul 23, 2014 at 5:17 PM, Zbigniew Jędrzejewski-Szmek
 wrote:
> On Wed, Jul 23, 2014 at 04:55:59PM +0200, Kay Sievers wrote:
>> On Wed, Jul 23, 2014 at 4:28 PM, Zbigniew Jędrzejewski-Szmek
>>  wrote:
>>
>> > Anyway, I think that /etc/login.defs support is made out to be something
>> > much more complicated than it really is. IMHO we should:
>> >
>> > - read /etc/login.defs and fall back to the compiled in value
>> > - use whatever result we get in coredump, journald, and sysusers
>> >
>> > It's not like the implementation would be hard, intrusive, or slow. It'd be
>> > probably +3 lines in one or two places.
>>
>> It is not about the effort *how* to do it, it is *why*. And I still
>> don't think we should have dynamic configuration options for this, it
>> is all just a huge mess that needs to be limited to the bare minimum,
>> it is just too broken as a concept to be supported that way.
>>
>> > If we come up with additional heuristics or rules to determine human
>> > accounts, we can safely add them in a backwards compatible way.
>>
>> We cannot do any normal user queries from journald, so none of the
>> metadata like the primary group is easily for a user is available.
> I know.
>
>> Sysusers is, and probably always will be, limited to the classic
>> passwd, group file. Maybe we can just read the files ourselves and
>> find out that a certain uid is a normal user? Like:
>>   - uid >= "1000" --> normal user
>>   - lookup uid in passwd
>>   - user not found --> normal user
>>   - user < 1000 && group == "users" --> normal user
>>   - everything else would be a system user
> But please add to this (at the top)
> - parse SYS_GID_MIN and SYS_GID_MAX from /etc/login.defs and if
>   found and users falls within --> system user
>
> This is safe as soon as /etc is accessible and provides backwards
> compatibillity.

Well, the point is to make the rules in this broken model simpler, not
more complicated as they already are. :)

If we would read login.defs, we should probably not do any magic
heuristics. And at the moment, I still think we should ignore
login.defs.

> Also, I'd modify
> - user < 1000 && group == "users" --> normal user
> to
> - group == "users" --> normal user
> not to make things too complicated.
>
> I see another angry chicken and broken egg problem now:
> - We want to get rid of /etc/login.defs, *but*
> - we read /etc/login.defs at compilation time.
> This means that we probably should stop looking at that file during
> compilation time and stick to an internal default, possibly allowing
> overriding with ./configure switch.

Maybe, yes. It was just to init the build sys with the current distro defaults.

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

Re: [systemd-devel] sysusers and login.defs checks

2014-07-23 Thread Zbigniew Jędrzejewski-Szmek

On Wed, Jul 23, 2014 at 04:55:59PM +0200, Kay Sievers wrote:
> On Wed, Jul 23, 2014 at 4:28 PM, Zbigniew Jędrzejewski-Szmek
>  wrote:
> 
> > Anyway, I think that /etc/login.defs support is made out to be something
> > much more complicated than it really is. IMHO we should:
> >
> > - read /etc/login.defs and fall back to the compiled in value
> > - use whatever result we get in coredump, journald, and sysusers
> >
> > It's not like the implementation would be hard, intrusive, or slow. It'd be
> > probably +3 lines in one or two places.
> 
> It is not about the effort *how* to do it, it is *why*. And I still
> don't think we should have dynamic configuration options for this, it
> is all just a huge mess that needs to be limited to the bare minimum,
> it is just too broken as a concept to be supported that way.
> 
> > If we come up with additional heuristics or rules to determine human
> > accounts, we can safely add them in a backwards compatible way.
> 
> We cannot do any normal user queries from journald, so none of the
> metadata like the primary group is easily for a user is available.
I know.

> Sysusers is, and probably always will be, limited to the classic
> passwd, group file. Maybe we can just read the files ourselves and
> find out that a certain uid is a normal user? Like:
>   - uid >= "1000" --> normal user
>   - lookup uid in passwd
>   - user not found --> normal user
>   - user < 1000 && group == "users" --> normal user
>   - everything else would be a system user
But please add to this (at the top)
- parse SYS_GID_MIN and SYS_GID_MAX from /etc/login.defs and if
  found and users falls within --> system user

This is safe as soon as /etc is accessible and provides backwards
compatibillity.

Also, I'd modify 
- user < 1000 && group == "users" --> normal user
to
- group == "users" --> normal user
not to make things too complicated.

I see another angry chicken and broken egg problem now:
- We want to get rid of /etc/login.defs, *but*
- we read /etc/login.defs at compilation time.
This means that we probably should stop looking at that file during
compilation time and stick to an internal default, possibly allowing
overriding with ./configure switch.

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

Re: [systemd-devel] sysusers and login.defs checks

2014-07-23 Thread Kay Sievers

On Wed, Jul 23, 2014 at 4:28 PM, Zbigniew Jędrzejewski-Szmek
 wrote:

> Anyway, I think that /etc/login.defs support is made out to be something
> much more complicated than it really is. IMHO we should:
>
> - read /etc/login.defs and fall back to the compiled in value
> - use whatever result we get in coredump, journald, and sysusers
>
> It's not like the implementation would be hard, intrusive, or slow. It'd be
> probably +3 lines in one or two places.

It is not about the effort *how* to do it, it is *why*. And I still
don't think we should have dynamic configuration options for this, it
is all just a huge mess that needs to be limited to the bare minimum,
it is just too broken as a concept to be supported that way.

> If we come up with additional heuristics or rules to determine human
> accounts, we can safely add them in a backwards compatible way.

We cannot do any normal user queries from journald, so none of the
metadata like the primary group is easily for a user is available.

Sysusers is, and probably always will be, limited to the classic
passwd, group file. Maybe we can just read the files ourselves and
find out that a certain uid is a normal user? Like:
  - uid >= "1000" --> normal user
  - lookup uid in passwd
  - user not found --> normal user
  - user < 1000 && group == "users" --> normal user
  - everything else would be a system user

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

Re: [systemd-devel] [PATCH 1/2] tools: add script to detect repeating words in docs

2014-07-23 Thread Karel Zak

On Wed, Jul 23, 2014 at 02:59:26PM +0200, Zbigniew Jędrzejewski-Szmek wrote:
> On Wed, Jul 23, 2014 at 02:57:10PM +0200, Zbigniew Jędrzejewski-Szmek wrote:
> > On Wed, Jul 23, 2014 at 12:40:06PM +0200, Karel Zak wrote:
> > >  - all  sections from input files are ignored
> > >  - it's possible to white-list wanted repeats by KNOWN_REPEATS[] in the 
> > > script
> > >  - the script is based on checkmans.sh from util-linux project
> > >  - it's integrated to build-sys, just type "make check-repwords", for 
> > > example:
> > Hi,
> > 
> > I think it would be nicer to simply incorporate the check into
> > tools/make-directive-index.py. It already contains some checks on
> Actually it's check_id() in make-man-index.py.

 ...but then it will be usable only for the xml->man stuff.
 
 Now the script is generic and you can use it for example for gtk-docs 
 too (just add another "tools/check-repwords.sh " into
 Makefile.am) or arbitrary another text.

Karel

-- 
 Karel Zak  
 http://karelzak.blogspot.com
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Changing configurations with networkd

2014-07-23 Thread Marcel Holtmann

Hi Michael,

>>> I think the lease should be remembered and reused in this case.
>> 
>> Hm, this sounds like a bug somewhere. When the new discover is sent
>> out it should send the same identifying information to the server, and
>> hence be given the same lease back again. Wireshark should tell you if
>> the fault is networkd's or the DHCP server.
> 
> I get the same address at first, but after several minutes the DHCP server
> decides to offer a new address. I should note, that I have a 10 minute
> lease time for debugging purposes, so that might make the problem more
> prominent. I'll see if I can figure out what happens here.

look at your DHCP server and see what lease time it really hands out after 
reboot.

However this is between you and your DHCP server. If you configure a lease time 
of 10 minutes, then that is the only guaranteed time for a given IP address. 
There is no mandate that the server has to give you the same address after 10 
minutes when you ask again. It is valid to just get a different one. And that 
many home routers try to give you back the same one does not mean that they are 
required to do so.

The nice DHCP servers will remember your Ethernet address and/or identity 
information and give you back your old IP address. Either with the left over 
lease time or with a brand new lease time. There is really no need to store 
this information on disk. If the lease expired the information on disk are 
stale as well. And since our DHCP implementation is so fast, it makes really no 
difference.

It is safer start out with a brand new DHCP lease instead of having to deal 
with renewal during boot. At least that way you know the DHCP server is still 
there and you have a valid IP address. Just re-using a stored IP with a 
left-over lease is not safe anyway. You never know what changed in the network 
when you were off.

Regards

Marcel

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

Re: [systemd-devel] sysusers and login.defs checks

2014-07-23 Thread Zbigniew Jędrzejewski-Szmek

On Wed, Jul 23, 2014 at 11:29:20AM +0100, Colin Guthrie wrote:
> 'Twas brillig, and Colin Walters at 22/07/14 21:47 did gyre and gimble:
> > On Mon, Jul 21, 2014, at 09:43 AM, Lennart Poettering wrote:
> >>
> >> I am pretty strongly against this. Making this administrator
> >> configurable apepars very wrong, this really should be a decision for
> >> the distribution vendor, and that's it.
> > 
> > You list one concern below, are there others?
> > 
> >>  We shouldn't design a system
> >> that comes to completely different results if you boot it up with and
> >> without /etc populated...
> > 
> > If that's the only issue, surely we could just have it in the
> > /usr/share/factory dir?
> 
> While I'm (personally) wanting to support login.defs here, there is
> still a chicken+egg that is not really resolvable here (AFAICT)
> 
> If there was a /usr/share/factory/etc/login.defs with e.g. 500 boundary
> point, then this file would presumably be copied in by tmpfiles to
> populate /etc/login.defs
I totally agree with Kay here, that /usr/share/factory is *only* for
programs which are broken and have neither compiled-in defaults nor
the ability to read /usr/share for their defaults. We should 
aim for empty /usr/share/factory/etc (*), and systemd certainly should
not depend or use anything in it.

And anyway, the file in /usr/share/factory/etc would have the same
defaults as compiled-in into systemd, so it is not useful. I think
that your angry chicken is not a problem.

(*) "Empty" files containing commented-out directives as a way of
documentation are fine.

> This was pretty much the same concern I had when Zbigniew patched
> sysusers to also look in /etc/sysusers.d/. The same chicken and egg
> scenario exists there, but Lennart was OK with it in that context.
> 
> To be honest, I'm not really sure why this chicken+egg is different from
> the sysusers one, so I'd expect either both to be rejected or both
> accepted but I'm not crazy fussed in this case as it should be easy
> enough to hack in /etc/login.defs parsing downstream where our primary
> use case is supporting "normal" installs where /etc/ is persistent and
> often upgraded from older releases.
The fallacy here is trying to support stateless systems with users.
Users (account numbers, their passwords, privileges) are state,
and the most important one at that. But the admin cannot expect to
nuke /etc and have working users, unless the users are defined
remotely.

I think it is fine to have locally defined users, and some way to
determine which of those users are "human". If the admin purges /etc,
the interpretation of which users are human resets to the compiled in
default. IMHO this is fine.

I like Lennarts idea of using 'user' group, since it is more flexible than
static ranges. It might be nice to support in the future if we work out
the details. But there's also a need for backwards compatibility. I know
from my own admin experience and agree with Reindl that UID changes are a
huge pain, and we have to support existing setups without requiring any
renumbering. This basically means that any distribution which supports
upgrades from previous versions will add /etc/login.defs support.

On Wed, Jul 23, 2014 at 02:22:59PM +0200, Kay Sievers wrote:
> No, I still think we should limit the options of the conceptually completely
> broken idea of unix uids to the absolute minimum, and not try to fix things
> in any way here with layering even more broken options on top.
Well, systemd uses the user/systemd boundary split in journald and coredump
permissions code, so we *are* layering on top. Even more importantly, we
are using it to allocate numbers in sysusers. Even with distribution
support, it is not possible to arbitrarily redefine UID ranges,
because users' UIDs are something that can be only changed by admin
intervention. I don't see how we can skip /etc/login.defs support
without doing a disservice to our users.

Previously we added the systemd-journal group. It was fine, because
it worked in a backwards-compatible way, and we could document that giving
users access to the journal can be done following a few simple steps.

Anyway, I think that /etc/login.defs support is made out to be something
much more complicated than it really is. IMHO we should:

- read /etc/login.defs and fall back to the compiled in value
- use whatever result we get in coredump, journald, and sysusers

It's not like the implementation would be hard, intrusive, or slow. It'd be
probably +3 lines in one or two places.

If we come up with additional heuristics or rules to determine human
accounts, we can safely add them in a backwards compatible way.

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

Re: [systemd-devel] [PATCH 1/2] tools: add script to detect repeating words in docs

2014-07-23 Thread Zbigniew Jędrzejewski-Szmek

On Wed, Jul 23, 2014 at 02:57:10PM +0200, Zbigniew Jędrzejewski-Szmek wrote:
> On Wed, Jul 23, 2014 at 12:40:06PM +0200, Karel Zak wrote:
> >  - all  sections from input files are ignored
> >  - it's possible to white-list wanted repeats by KNOWN_REPEATS[] in the 
> > script
> >  - the script is based on checkmans.sh from util-linux project
> >  - it's integrated to build-sys, just type "make check-repwords", for 
> > example:
> Hi,
> 
> I think it would be nicer to simply incorporate the check into
> tools/make-directive-index.py. It already contains some checks on
Actually it's check_id() in make-man-index.py.

>  header correctness. Since it already parses all XML, you'd
> get this check basically for free run-time wise, and it should be much
> simpler to write and more robust, since you don't have to parse XML by hand 
> [1].
> 
> Zbyszek
> 
> [1] http://lxml.de/api/lxml.etree-module.html#strip_tags

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

Re: [systemd-devel] [PATCH 1/2] tools: add script to detect repeating words in docs

2014-07-23 Thread Zbigniew Jędrzejewski-Szmek

On Wed, Jul 23, 2014 at 12:40:06PM +0200, Karel Zak wrote:
>  - all  sections from input files are ignored
>  - it's possible to white-list wanted repeats by KNOWN_REPEATS[] in the script
>  - the script is based on checkmans.sh from util-linux project
>  - it's integrated to build-sys, just type "make check-repwords", for example:
Hi,

I think it would be nicer to simply incorporate the check into
tools/make-directive-index.py. It already contains some checks on
 header correctness. Since it already parses all XML, you'd
get this check basically for free run-time wise, and it should be much
simpler to write and more robust, since you don't have to parse XML by hand [1].

Zbyszek

[1] http://lxml.de/api/lxml.etree-module.html#strip_tags
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 2/2] docs: remove repeating words from man/*xml

2014-07-23 Thread Zbigniew Jędrzejewski-Szmek

On Wed, Jul 23, 2014 at 12:40:07PM +0200, Karel Zak wrote:
> ---
>  man/coredump.conf.xml   | 2 +-
>  man/sd_bus_message_append_array.xml | 2 +-
>  man/systemctl.xml   | 2 +-
>  man/systemd-journal-remote.xml  | 2 +-
>  man/systemd.journal-fields.xml  | 2 +-
>  man/sysusers.d.xml  | 2 +-
>  6 files changed, 6 insertions(+), 6 deletions(-)
Applied 2/2.

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

Re: [systemd-devel] Changing configurations with networkd

2014-07-23 Thread Michael Olbrich

On Wed, Jul 23, 2014 at 12:47:37PM +0200, Tom Gundersen wrote:
> On Wed, Jul 23, 2014 at 9:50 AM, Michael Olbrich
>  wrote:
> > I've been experimenting with systemd-networkd to see where it fits my
> > use-cases. I'm looking for some insight if the issues I'm seeing are bugs,
> > features just not implemented yet or if my use-case is out of scope for
> > networkd.
> > The most common use-case I have is rather simple: One ethernet interface
> > that can be configured either with a static IP address or via DHCP. The
> > configuration usually comes from some custom configuration data, so I'll
> > probably have a default config as part of the rootfs and generate the real
> > config during startup in /run. This works rather well and is much nicer
> > than some custom shell scripts.
> > The interesting part is, when the configuration changes: I'm working with
> > embedded devices and network access is rarely part of the primary function
> > of the device. As a result, rebooting the device to change the network
> > config is not acceptable.
> >
> > So I tried to change a static IP address by changing the config and
> > restarting networkd. The result was an interface with two addresses. And
> > worse, the old address was still the preferred on.
> > I realize, that a smooth transition is probably impossible for complex
> > configuration possibilities of networkd. But I don't need smooth. How about
> > optionally shutting down an interface when networkd stops?
> > ShutdownWithNetworkd=yes or something like that in the config file?
> 
> Dynamic changing of settings has not been implemented yet, we are so
> far only focussing on the static setups. We definitely will cover more
> dynamic things in the future though, such as your usecase.
> 
> We'll have a dbus interface, and not use stop/restart as a
> configuration mechanism, so I don't think tearing things down on
> shutdown of the demon makes much sense.

That sounds interesting.

> In the meantime, you could hack this by doing:
> 
> systemctl stop systemd-networkd
> ip link address flush dev 
> systemctl start systemd-networkd

I know, but that's not very nice, and I need to handle nfs-root special
cases manually...

> > Another thing I noticed is with DHCP without changing the configuration:
> > Networkd forgets the lease during restart and sends a dhcp discover. Then
> > the server might offer a different address. The problem is, that the new
> > address is in the same subnet as the old address, so it is added as
> > secondary address. When the valid_lft of the old address expires then
> > _both_ addresses are removed :-/.
> 
> Thanks for this report, we should figure out a way that the new
> address is not lost when the first one is.

I looked at the code, but I need to learn more about netlink before I
understand the little details there.

> > I think the lease should be remembered and reused in this case.
> 
> Hm, this sounds like a bug somewhere. When the new discover is sent
> out it should send the same identifying information to the server, and
> hence be given the same lease back again. Wireshark should tell you if
> the fault is networkd's or the DHCP server.

I get the same address at first, but after several minutes the DHCP server
decides to offer a new address. I should note, that I have a 10 minute
lease time for debugging purposes, so that might make the problem more
prominent. I'll see if I can figure out what happens here.

Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] systemd-socket-proxyd & slapd

2014-07-23 Thread Zbigniew Jędrzejewski-Szmek

On Wed, Jul 23, 2014 at 11:13:50AM +0300, Suvendu Mitra wrote:
> Thanks now it works, But does it mandatory to start slapd on same port as
> "ListenStream=" of socket file of systemd-socket-proxyd. e.g in following
> example port 400.
Maybe the protocol embeds the port number in the stream? That wouldn't be
so unusual, HTML does such braindead things too.

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

Re: [systemd-devel] [PATCH] coredump: suppress uninitialized sz warning

2014-07-23 Thread Zbigniew Jędrzejewski-Szmek

On Wed, Jul 23, 2014 at 10:02:57AM +0200, Daniel Buch wrote:
> Its false positive but lets make gcc happy
We have approximately a million of those... If you compile with -O2 or -O3 they
really multiply. I filed https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61846,
so let's wait for the upstream response before applying such patches.

Zbyszek

> diff --git a/src/journal/coredump.c b/src/journal/coredump.c
> index 182c2b1..a361a51 100644
> --- a/src/journal/coredump.c
> +++ b/src/journal/coredump.c
> @@ -700,7 +700,7 @@ log:
>  /* Optionally store the entire coredump in the journal */
>  if (IN_SET(arg_storage, COREDUMP_STORAGE_JOURNAL, 
> COREDUMP_STORAGE_BOTH) &&
>  coredump_size <= (off_t) arg_journal_size_max) {
> -size_t sz;
> +size_t sz = 0;
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] sysusers and login.defs checks

2014-07-23 Thread Kay Sievers

On Wed, Jul 23, 2014 at 1:49 PM, Colin Guthrie  wrote:
> Kay Sievers wrote on 23/07/14 12:36:
>> I don't see the rather artificially constructed case of an
>> /usr/share/factory/etc/login.defs + tmpfiles snippet to copy to /etc
>> as a valid argument for reading login.defs.
>
> Well, my point was that one of Lennart's original arguments for NOT
> reading login.defs was that /etc/ wouldn't have it when bootstrapping.

The main point was that it should not be *configurable*, but be a *static*
OS property.

Unix/Linux is just so insufficient and broken regarding the uid
namespace and handling, that we should limit ourselves to the simplest
rules possible and live with it, instead of trying to fix something that is so
broken that it can never work properly with all the options people like to
throw at it.

> But as you've just confirmed if you *are* bootstrapping /etc, then using
> the compiled in defaults makes sense as you are very unlikely to be
> copying across a factory version of login.defs anyway. My point was that
> if you WERE copying across of a factory version of login.defs it should
> really be setup with the same defaults that systemd has compiled in anyway.

Right, anything else would just be broken. Copying login.defs at bootstrap time
would not make any sense in the first place. But, hey, it would be just one of
thousand ways to break a setup. It's nothing we need to care about.

> So again, this is just furthering the argument that we *should* read
> /etc/login.defs at runtime and the "bootstrapping" argument for not
> doing so is not really valid.

No, I still think we should limit the options of the conceptually completely
broken idea of unix uids to the absolute minimum, and not try to fix things
in any way here with layering even more broken options on top.

If we want uids that have meaning and can be managed, they should
become 128 (uuid) or at least 64 bit, so they can be statically
allocated and moved
from one machine to another.

The sysuser vs. normal user is really not significantly more that a convenience
feature, it is not used for authorization or anything. Legacy setups
on new systems
might end up with fewer convenience features, but there are no
features lost really,
which they had before systemd was used.

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

Re: [systemd-devel] sysusers and login.defs checks

2014-07-23 Thread Colin Guthrie

Kay Sievers wrote on 23/07/14 12:36:
> I don't see the rather artificially constructed case of an
> /usr/share/factory/etc/login.defs + tmpfiles snippet to copy to /etc
> as a valid argument for reading login.defs.

Well, my point was that one of Lennart's original arguments for NOT
reading login.defs was that /etc/ wouldn't have it when bootstrapping.

But as you've just confirmed if you *are* bootstrapping /etc, then using
the compiled in defaults makes sense as you are very unlikely to be
copying across a factory version of login.defs anyway. My point was that
if you WERE copying across of a factory version of login.defs it should
really be setup with the same defaults that systemd has compiled in anyway.

So again, this is just furthering the argument that we *should* read
/etc/login.defs at runtime and the "bootstrapping" argument for not
doing so is not really valid.

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] sysusers and login.defs checks

2014-07-23 Thread Kay Sievers

On Wed, Jul 23, 2014 at 1:11 PM, Colin Guthrie  wrote:
> 'Twas brillig, and Colin Guthrie at 23/07/14 11:29 did gyre and gimble:
>> If there was a /usr/share/factory/etc/login.defs with e.g. 500 boundary
>> point, then this file would presumably be copied in by tmpfiles to
>> populate /etc/login.defs
>
> Of course one thing that makes this argument slightly invalid is that if
> you DO have a /usr/share/factory/etc/login.defs, then this file *should*
> be configured with the same boundary as systemd's built in defaults, so
> there should be no problem in the context of bootstrapping /etc.
>
> Therefore, if this is the case, then the argument for not reading and
> honouring /etc/login.defs if it exists is, IMO, invalid.
>
> Have I won Lennart? Are you now convinced that such a patch would be in
> the same category as Zbigniew's sysusers reading /etc/sysusers.d/ patch
> that already went in? :p

I don't see the rather artificially constructed case of an
/usr/share/factory/etc/login.defs + tmpfiles snippet to copy to /etc
as a valid argument for reading login.defs.

Configuring our own base bootstrap tools with /usr/share/factory/ + a
tmpfiles snippet to copy the stuff over to /etc is a cyclic loop we
just do not support, and which makes not much sense anyway.

Bootstrap-/etc-population is intended for "broken" tools that cannot
be fixed properly, not our own stuff, which does not need /etc
populated.

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

Re: [systemd-devel] sysusers and login.defs checks

2014-07-23 Thread Colin Guthrie

'Twas brillig, and Colin Guthrie at 23/07/14 11:29 did gyre and gimble:
> If there was a /usr/share/factory/etc/login.defs with e.g. 500 boundary
> point, then this file would presumably be copied in by tmpfiles to
> populate /etc/login.defs

Of course one thing that makes this argument slightly invalid is that if
you DO have a /usr/share/factory/etc/login.defs, then this file *should*
be configured with the same boundary as systemd's built in defaults, so
there should be no problem in the context of bootstrapping /etc.

Therefore, if this is the case, then the argument for not reading and
honouring /etc/login.defs if it exists is, IMO, invalid.

Have I won Lennart? Are you now convinced that such a patch would be in
the same category as Zbigniew's sysusers reading /etc/sysusers.d/ patch
that already went in? :p

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

[systemd-devel] [PATCH] sysv: order initscripts which provide $network before network.target

2014-07-23 Thread Lukas Nykryn

Due to recent changes where $network "maps" to network-online.target
it is not guaranteed that initscript which provides networking will
be terminated after network.target during shutdown which is against LSB.
---
 src/sysv-generator/sysv-generator.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/sysv-generator/sysv-generator.c 
b/src/sysv-generator/sysv-generator.c
index 5206279..9a869ba 100644
--- a/src/sysv-generator/sysv-generator.c
+++ b/src/sysv-generator/sysv-generator.c
@@ -482,6 +482,11 @@ static int load_sysv(SysvStub *s) {
 r = strv_extend(&s->wants, m);
 if (r < 0)
 return log_oom();
+if (streq(m, 
SPECIAL_NETWORK_ONLINE_TARGET)) {
+r = 
strv_extend(&s->before, SPECIAL_NETWORK_TARGET);
+if (r < 0)
+return 
log_oom();
+}
 }
 
 if (r < 0)
-- 
1.8.3.1

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

Re: [systemd-devel] sysusers and login.defs checks

2014-07-23 Thread Reindl Harald


Am 22.07.2014 22:47, schrieb Colin Walters:
> On Mon, Jul 21, 2014, at 09:43 AM, Lennart Poettering wrote:
>>
>> I am pretty strongly against this. Making this administrator
>> configurable apepars very wrong, this really should be a decision for
>> the distribution vendor, and that's it.
> 
> You list one concern below, are there others?
> 
>>  We shouldn't design a system
>> that comes to completely different results if you boot it up with and
>> without /etc populated...
> 
> If that's the only issue, surely we could just have it in the
> /usr/share/factory dir?
> 
> As far as the rationale for having it administrator configurable - I
> think the idea is more that upgraded systems have a login.defs file with
> a min of 500, so humans in the midrange are still identified as such.
> 
> This is called out on
> http://fedoraproject.org/wiki/Features/1000SystemAccounts
> 
> Making the boundary configurable also allows some users to stay with the
> old boundary of 500, if they wish:
> 
> Because /etc/login.defs is %config(noreplace), upgrades will retain
> the boundary value 500, and nothing should break.
> New installations in setups where the UIDs are centrally allocated
> (e.g. using LDAP) from 500 could be likewise configured to use the
> boundary value 500 by creating /etc/login.defs in a kickstart %pre
> script

there are *a lot* of systems out there installed long before systemd
was introduced and you can't safely dig around on dozens of machines
and re-assign the owners of files

there are data far away from /home

don't fix things which ain't broken
all that machines will *never* need dynamic user-id's abvoe 500



signature.asc
Description: OpenPGP digital signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Changing configurations with networkd

2014-07-23 Thread Reindl Harald


Am 23.07.2014 12:47, schrieb Tom Gundersen:
>> I think the lease should be remembered and reused in this case.
> In general, saving the lease to disk is probably not a good idea. We
> would need to store it on /var, and we may need to start the DHCP
> server before /var has been mounted. To the extent possible I would
> prefer to avoid that problem

wheren't such cases the reason to invent /run?



signature.asc
Description: OpenPGP digital signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Changing configurations with networkd

2014-07-23 Thread Tom Gundersen

On Wed, Jul 23, 2014 at 9:50 AM, Michael Olbrich
 wrote:
> I've been experimenting with systemd-networkd to see where it fits my
> use-cases. I'm looking for some insight if the issues I'm seeing are bugs,
> features just not implemented yet or if my use-case is out of scope for
> networkd.
> The most common use-case I have is rather simple: One ethernet interface
> that can be configured either with a static IP address or via DHCP. The
> configuration usually comes from some custom configuration data, so I'll
> probably have a default config as part of the rootfs and generate the real
> config during startup in /run. This works rather well and is much nicer
> than some custom shell scripts.
> The interesting part is, when the configuration changes: I'm working with
> embedded devices and network access is rarely part of the primary function
> of the device. As a result, rebooting the device to change the network
> config is not acceptable.
>
> So I tried to change a static IP address by changing the config and
> restarting networkd. The result was an interface with two addresses. And
> worse, the old address was still the preferred on.
> I realize, that a smooth transition is probably impossible for complex
> configuration possibilities of networkd. But I don't need smooth. How about
> optionally shutting down an interface when networkd stops?
> ShutdownWithNetworkd=yes or something like that in the config file?

Dynamic changing of settings has not been implemented yet, we are so
far only focussing on the static setups. We definitely will cover more
dynamic things in the future though, such as your usecase.

We'll have a dbus interface, and not use stop/restart as a
configuration mechanism, so I don't think tearing things down on
shutdown of the demon makes much sense.

In the meantime, you could hack this by doing:

systemctl stop systemd-networkd
ip link address flush dev 
systemctl start systemd-networkd

> Another thing I noticed is with DHCP without changing the configuration:
> Networkd forgets the lease during restart and sends a dhcp discover. Then
> the server might offer a different address. The problem is, that the new
> address is in the same subnet as the old address, so it is added as
> secondary address. When the valid_lft of the old address expires then
> _both_ addresses are removed :-/.

Thanks for this report, we should figure out a way that the new
address is not lost when the first one is.

> I think the lease should be remembered and reused in this case.

Hm, this sounds like a bug somewhere. When the new discover is sent
out it should send the same identifying information to the server, and
hence be given the same lease back again. Wireshark should tell you if
the fault is networkd's or the DHCP server.

In general, saving the lease to disk is probably not a good idea. We
would need to store it on /var, and we may need to start the DHCP
server before /var has been mounted. To the extent possible I would
prefer to avoid that problem.

Cheers

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

[systemd-devel] [PATCH 1/2] tools: add script to detect repeating words in docs

2014-07-23 Thread Karel Zak

 - all  sections from input files are ignored
 - it's possible to white-list wanted repeats by KNOWN_REPEATS[] in the script
 - the script is based on checkmans.sh from util-linux project
 - it's integrated to build-sys, just type "make check-repwords", for example:

$ make check-repwords
  GEN  check-repwords
warning: man/coredump.conf.xml has repeating words: on
warning: man/sd_bus_message_append_array.xml has repeating words: of
warning: man/systemctl.xml has repeating words: on
warning: man/systemd.journal-fields.xml has repeating words: with
warning: man/systemd-journal-remote.xml has repeating words: is
warning: man/sysusers.d.xml has repeating words: be
---
 Makefile.am |  7 
 tools/check-repwords.sh | 97 +
 2 files changed, 104 insertions(+)
 create mode 100755 tools/check-repwords.sh

diff --git a/Makefile.am b/Makefile.am
index 3fb3703..7ee0264 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -5918,6 +5918,13 @@ check-includes: $(top_srcdir)/tools/check-includes.pl
 EXTRA_DIST += \
$(top_srcdir)/tools/check-includes.pl
 
+.PHONY: check-repwords
+check-repwords: $(top_srcdir)/tools/check-repwords.sh
+   $(AM_V_GEN) $(top_srcdir)/tools/check-repwords.sh man/*.xml
+
+EXTRA_DIST += \
+   $(top_srcdir)/tools/check-repwords.sh
+
 # Stupid test that everything purported to be exported really is
 define generate-sym-test
$(AM_V_at)$(MKDIR_P) $(dir $@)
diff --git a/tools/check-repwords.sh b/tools/check-repwords.sh
new file mode 100755
index 000..f2aa327
--- /dev/null
+++ b/tools/check-repwords.sh
@@ -0,0 +1,97 @@
+#!/bin/bash
+#
+# Check all files specified on command line for repeating words
+#
+# Copyright (C) 2014 Karel Zak 
+# based on util-linux checkmans.sh from Sami Kerola 
+#
+
+set -e # exit on errors
+set -o pipefail# exit if pipe writer fails
+set -u # disallow usage of unset variables
+set -C # disallow redirection file overwriting
+SCRIPT_INVOCATION_SHORT_NAME=$(basename ${0})
+trap 'echo "${SCRIPT_INVOCATION_SHORT_NAME}: exit on error"; exit 1' ERR
+
+usage() {
+   echo "Usage: ${0} [-vVh] "
+   echo " -v  verbose messaging"
+   echo " -h  print this help and exit"
+}
+
+VERBOSE='false'
+while getopts vh OPTIONS; do
+   case ${OPTIONS} in
+   v)
+   VERBOSE='true'
+   ;;
+   h)
+   usage
+   exit 0
+   ;;
+   *)
+   usage
+   exit 1
+   esac
+done
+
+shift $(( OPTIND - 1 ))
+
+declare -a REPEATS
+declare -A KNOWN_REPEATS
+
+### white list
+# Note that all text between   tags is 
ingored.
+#
+# For exmaple to ignore 'bar bar' in the file foo.xml define:
+# KNOWN_REPEATS[foo.xml]='bar'
+
+
+remove_repeats()
+{
+   set +u
+   for KN in ${KNOWN_REPEATS[${I##*/}]}; do
+   if [ "${KN}" = "${REPEATS[$1]}" ]; then
+   if $VERBOSE; then
+   echo "info: ${I} ignore repeat: ${REPEATS[$1]}"
+   fi
+   unset REPEATS[$1]
+   fi
+   done
+   set -u
+}
+
+COUNT_ERRORS=0
+
+for I in $*; do
+   I_ERR=0
+   if ${VERBOSE}; then
+   echo "testing: ${I}"
+   fi
+   REPEATS=( $( cat ${I} | col -b | \
+   sed  -e 's/\s\+/\n/g;
+/^$/d;
+//d' | \
+   awk 'BEGIN { p="" } { if (0 < length($0)) { if (p == $0) { 
print } } p = $0 }') )
+
+   if [ 0 -lt "${#REPEATS[@]}" ]; then
+   ITER=${#REPEATS[@]}
+   while [ -1 -lt ${ITER} ]; do
+   remove_repeats ${ITER}
+   # The 'let' may cause exit on error.
+   # When ITER == 0 -> let returns 1, bash bug?
+   let ITER=${ITER}-1 || true
+   done
+   if [ 0 -lt "${#REPEATS[@]}" ]; then
+   echo "warning: ${I} has repeating words: ${REPEATS[@]}"
+   fi
+   fi
+
+   let COUNT_ERRORS=$COUNT_ERRORS+$I_ERR || true
+done
+
+if [ ${COUNT_ERRORS} -ne 0 ]; then
+   exit 1
+fi
+
+exit 0
-- 
1.9.3

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

[systemd-devel] repeating words

2014-07-23 Thread Karel Zak

Not sure if the script from the first patch is wanted, but at least
the second patch that fix typos in man/*.xml files should be applied.

 [PATCH 1/2] tools: add script to detect repeating words in docs
 [PATCH 2/2] docs: remove repeating words from man/*xml

 Karel

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

[systemd-devel] [PATCH 2/2] docs: remove repeating words from man/*xml

2014-07-23 Thread Karel Zak

---
 man/coredump.conf.xml   | 2 +-
 man/sd_bus_message_append_array.xml | 2 +-
 man/systemctl.xml   | 2 +-
 man/systemd-journal-remote.xml  | 2 +-
 man/systemd.journal-fields.xml  | 2 +-
 man/sysusers.d.xml  | 2 +-
 6 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/man/coredump.conf.xml b/man/coredump.conf.xml
index fa55eb9..0c9160e 100644
--- a/man/coredump.conf.xml
+++ b/man/coredump.conf.xml
@@ -126,7 +126,7 @@
 to 15% of the total disk size). Note that the disk space used
 by coredumps might temporarily exceed these limits while
 coredumps are processed. Note that old coredumps are also
-removed based on on time via
+removed based on time via
 
systemd-tmpfiles8.
   
 
diff --git a/man/sd_bus_message_append_array.xml 
b/man/sd_bus_message_append_array.xml
index bf40da8..e0f6767 100644
--- a/man/sd_bus_message_append_array.xml
+++ b/man/sd_bus_message_append_array.xml
@@ -104,7 +104,7 @@ along with systemd; If not, see 
.
 http://dbus.freedesktop.org/doc/dbus-specification.html#basic-types";>Basic 
Types
 section of the D-Bus specification, and listed in
 
sd_bus_message_append_basic3.
-Pointer p must point to an array of of size
+Pointer p must point to an array of size
 size bytes containing items of the
 respective type. Size size must be a
 multiple of the size of the type type. As a
diff --git a/man/systemctl.xml b/man/systemctl.xml
index 155dab3..a8d15b5 100644
--- a/man/systemctl.xml
+++ b/man/systemctl.xml
@@ -1274,7 +1274,7 @@ kobject-uevent 1 systemd-udevd-kernel.socket 
systemd-udevd.service
 Reload systemd manager configuration. This will reload
 all unit files and recreate the entire dependency
 tree. While the daemon is being reloaded, all sockets systemd
-listens on on behalf of user configuration will stay
+listens on behalf of user configuration will stay
 accessible. This command should not be confused
 with the load or
 reload commands.
diff --git a/man/systemd-journal-remote.xml b/man/systemd-journal-remote.xml
index b470e2c..08c0283 100644
--- a/man/systemd-journal-remote.xml
+++ b/man/systemd-journal-remote.xml
@@ -221,7 +221,7 @@ along with systemd; If not, see 
.
 will be created underneath the selected directory. Files will be
 called
 remote-hostname.journal,
-where the hostname part is is the
+where the hostname part is the
 escaped hostname of the source endpoint of the connection, or the
 numerical address if the hostname cannot be determined.
 
diff --git a/man/systemd.journal-fields.xml b/man/systemd.journal-fields.xml
index 5e12e61..154b95a 100644
--- a/man/systemd.journal-fields.xml
+++ b/man/systemd.journal-fields.xml
@@ -582,7 +582,7 @@
 microseconds, formatted as a decimal
 string. To be useful as an
 address for the entry, this
-should be combined with with the
+should be combined with the
 boot ID in 
_BOOT_ID=.
 
 
diff --git a/man/sysusers.d.xml b/man/sysusers.d.xml
index 1e079b2..58f24a6 100644
--- a/man/sysusers.d.xml
+++ b/man/sysusers.d.xml
@@ -138,7 +138,7 @@ m authd input
 Name
 
 The name field specifies the user or
-group name. It should be be shorter than 31
+group name. It should be shorter than 31
 characters and avoid any non-ASCII characters,
 and not begin with a numeric character. It is
 strongly recommended to pick user and group
-- 
1.9.3

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

Re: [systemd-devel] sysusers and login.defs checks

2014-07-23 Thread Colin Guthrie

'Twas brillig, and Colin Walters at 22/07/14 21:47 did gyre and gimble:
> On Mon, Jul 21, 2014, at 09:43 AM, Lennart Poettering wrote:
>>
>> I am pretty strongly against this. Making this administrator
>> configurable apepars very wrong, this really should be a decision for
>> the distribution vendor, and that's it.
> 
> You list one concern below, are there others?
> 
>>  We shouldn't design a system
>> that comes to completely different results if you boot it up with and
>> without /etc populated...
> 
> If that's the only issue, surely we could just have it in the
> /usr/share/factory dir?

While I'm (personally) wanting to support login.defs here, there is
still a chicken+egg that is not really resolvable here (AFAICT)

If there was a /usr/share/factory/etc/login.defs with e.g. 500 boundary
point, then this file would presumably be copied in by tmpfiles to
populate /etc/login.defs

BUT, as tmpfiles often needs users to exist (to do chowning etc), it has
to run after sysusers.

As sysusers would be one of the consumers of a /etc/login.defs parser,
it wouldn't "see" the /etc/login.defs until too late, thus possibly
creating system users with the compiled in default rather than the
"configured" default.

One cracked egg and an angry chicken :)

This was pretty much the same concern I had when Zbigniew patched
sysusers to also look in /etc/sysusers.d/. The same chicken and egg
scenario exists there, but Lennart was OK with it in that context.

To be honest, I'm not really sure why this chicken+egg is different from
the sysusers one, so I'd expect either both to be rejected or both
accepted but I'm not crazy fussed in this case as it should be easy
enough to hack in /etc/login.defs parsing downstream where our primary
use case is supporting "normal" installs where /etc/ is persistent and
often upgraded from older releases.

Col

PS I appreciate that I've "flipped sides" in my chicken+egg debate
compared to the concern I raised with Zbigniew's sysusers patch! I
always reserve the right to contradict myself :p

-- 

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] improve lid switch event handling

2014-07-23 Thread Lennart Poettering

On Tue, 22.07.14 12:35, Thomas Blume (thomas.bl...@suse.com) wrote:

> If so, we could record a previous lid open event e.g. in a status file.
> We could then inhibit the suspend if there was no previous lid open event or
>  allow it without timeout, if there was one.

Not following here... Note that the lid switch is actually an input
"switch" exposed by the kernel, not a "key". This means that it is a
binary thing anyway, and lid switch close events are never generated
when the device is already closed...

I am also pretty sure we should always react to lid switch state. Many
laptops have a power button accessible even if the device is closed (in
particular all "convertible" devices, such as the Lenovo Yoga I
have). Devices like that are prone to "accidental" resumes in backpacks
and such (because something presses on the power button), and we want
them to go back to suspend again, so that they don't stay on forever in
the backpack.

It's really annoying that we have to employ a timeout though before
going back to suspend. This is required because there's no point in time
when USB devices have to have shown up, and we want to allow USB docking
stations with VGA to block the suspend...

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-socket-proxyd & slapd

2014-07-23 Thread Suvendu Mitra

Thanks now it works, But does it mandatory to start slapd on same port as
"ListenStream=" of socket file of systemd-socket-proxyd. e.g in following
example port 400.

# cat proxy-to-directory-400.service
[Unit]
Requires=master-ldap-400.service
After=master-ldap-400.service

[Service]
ExecStart=/usr/lib/systemd/systemd-socket-proxyd ${HOSTNAME}:400
PrivateTmp=yes
PrivateNetwork=yes

# cat proxy-to-directory-400.socket
[Socket]
ListenStream=400

[Install]
WantedBy=sockets.target
---
#cat vgp.master-ldap-400.service
...
ExecStart=/usr/local/libexec/slapd -d 0 -f conf_400.conf -h
"ldap://${HOSTNAME}:400"; -l "LOCAL1"
...
---



On Thu, Jul 17, 2014 at 11:08 PM, David Timothy Strauss <
da...@davidstrauss.net> wrote:

> On Thu, Jul 17, 2014 at 4:51 AM, Zbigniew Jędrzejewski-Szmek
>  wrote:
> > I'd try without Private* settings.
> >
> > Also, replace /usr/lib/systemd/systemd-socket-proxyd with
> > '/bin/strace -o /tmp/log /usr/lib/systemd/systemd-socket-proxyd'
> > and look at the log file.
>
> Yes, get it working without Private*= first. JoinsNamespaceOf= is
> simply useful if you want to add it back after it's working.
>
> Another thing to try is targeting systemd-socket-proxyd at a different
> target (like a public website) to make sure it's activating properly.
> Also try connecting directly to the directory service without going
> through the proxy.
>



-- 
Suvendu Mitra
GSM - +358504821066
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] coredump: suppress uninitialized sz warning

2014-07-23 Thread Daniel Buch

Its false positive but lets make gcc happy
---
 src/journal/coredump.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/journal/coredump.c b/src/journal/coredump.c
index 182c2b1..a361a51 100644
--- a/src/journal/coredump.c
+++ b/src/journal/coredump.c
@@ -700,7 +700,7 @@ log:
 /* Optionally store the entire coredump in the journal */
 if (IN_SET(arg_storage, COREDUMP_STORAGE_JOURNAL, 
COREDUMP_STORAGE_BOTH) &&
 coredump_size <= (off_t) arg_journal_size_max) {
-size_t sz;
+size_t sz = 0;
 
 /* Store the coredump itself in the journal */
 
-- 
2.0.2

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

Re: [systemd-devel] crypttab automount

2014-07-23 Thread Jan

Ralf Jung  ralfj.de> writes:

> Essentially, I want a proper mount with the usual RequiredBy and
> WantedBy - but without the Before that makes others wait on this disk.
> So, the concurrency part of automount is exactly what I want, but
> without the on-demand part. Is that possible?

You're looking for "nofail" without the noauto (I think) (again in man
crypttab).



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

[systemd-devel] Changing configurations with networkd

2014-07-23 Thread Michael Olbrich

Hi,

I've been experimenting with systemd-networkd to see where it fits my
use-cases. I'm looking for some insight if the issues I'm seeing are bugs,
features just not implemented yet or if my use-case is out of scope for
networkd.
The most common use-case I have is rather simple: One ethernet interface
that can be configured either with a static IP address or via DHCP. The
configuration usually comes from some custom configuration data, so I'll
probably have a default config as part of the rootfs and generate the real
config during startup in /run. This works rather well and is much nicer
than some custom shell scripts.
The interesting part is, when the configuration changes: I'm working with
embedded devices and network access is rarely part of the primary function
of the device. As a result, rebooting the device to change the network
config is not acceptable.

So I tried to change a static IP address by changing the config and
restarting networkd. The result was an interface with two addresses. And
worse, the old address was still the preferred on.
I realize, that a smooth transition is probably impossible for complex
configuration possibilities of networkd. But I don't need smooth. How about
optionally shutting down an interface when networkd stops?
ShutdownWithNetworkd=yes or something like that in the config file?

Another thing I noticed is with DHCP without changing the configuration:
Networkd forgets the lease during restart and sends a dhcp discover. Then
the server might offer a different address. The problem is, that the new
address is in the same subnet as the old address, so it is added as
secondary address. When the valid_lft of the old address expires then
_both_ addresses are removed :-/.
I think the lease should be remembered and reused in this case.

Regards,
Michael

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] dhcp-network: remove unused DHCP6_STATE_RS

2014-07-23 Thread Tom Gundersen

Good catch. Applied.

Thanks.


Tom

On Wed, Jul 23, 2014 at 12:18 AM, Dan Williams  wrote:
> Probably a left-over from when router solicitations were
> requested in the DHCP6 code.  But since they are now separate,
> this state is no longer needed.
>
> Signed-off-by: Dan Williams 
> ---
>  src/libsystemd-network/dhcp6-protocol.h  | 1 -
>  src/libsystemd-network/sd-dhcp6-client.c | 4 
>  2 files changed, 5 deletions(-)
>
> diff --git a/src/libsystemd-network/dhcp6-protocol.h 
> b/src/libsystemd-network/dhcp6-protocol.h
> index e9ae598..eaa6717 100644
> --- a/src/libsystemd-network/dhcp6-protocol.h
> +++ b/src/libsystemd-network/dhcp6-protocol.h
> @@ -67,15 +67,14 @@ enum {
>  DHCP6_DUID_EN   = 2,
>  DHCP6_DUID_LL   = 3,
>  DHCP6_DUID_UUID = 4,
>  };
>
>  enum DHCP6State {
>  DHCP6_STATE_STOPPED = 0,
> -DHCP6_STATE_RS  = 1,
>  DHCP6_STATE_SOLICITATION= 2,
>  DHCP6_STATE_REQUEST = 3,
>  DHCP6_STATE_BOUND   = 4,
>  DHCP6_STATE_RENEW   = 5,
>  DHCP6_STATE_REBIND  = 6,
>  };
>
> diff --git a/src/libsystemd-network/sd-dhcp6-client.c 
> b/src/libsystemd-network/sd-dhcp6-client.c
> index 4f60578..13bed67 100644
> --- a/src/libsystemd-network/sd-dhcp6-client.c
> +++ b/src/libsystemd-network/sd-dhcp6-client.c
> @@ -289,15 +289,14 @@ static int client_send_message(sd_dhcp6_client *client) 
> {
>  r = dhcp6_option_append_ia(&opt, &optlen, 
> &client->lease->ia);
>  if (r < 0)
>  return r;
>
>  break;
>
>  case DHCP6_STATE_STOPPED:
> -case DHCP6_STATE_RS:
>  case DHCP6_STATE_BOUND:
>  return -EINVAL;
>  }
>
>  r = dhcp6_option_append(&opt, &optlen, DHCP6_OPTION_ORO,
>  client->req_opts_len * sizeof(be16_t),
>  client->req_opts);
> @@ -442,15 +441,14 @@ static int client_timeout_resend(sd_event_source *s, 
> uint64_t usec,
>  }
>  max_retransmit_duration = expire * USEC_PER_SEC;
>  }
>
>  break;
>
>  case DHCP6_STATE_STOPPED:
> -case DHCP6_STATE_RS:
>  case DHCP6_STATE_BOUND:
>  return 0;
>  }
>
>  if (max_retransmit_count &&
>  client->retransmit_count >= max_retransmit_count) {
>  client_stop(client, DHCP6_EVENT_RETRANS_MAX);
> @@ -839,15 +837,14 @@ static int client_receive_message(sd_event_source *s, 
> int fd, uint32_t revents,
>  break;
>
>  case DHCP6_STATE_BOUND:
>
>  break;
>
>  case DHCP6_STATE_STOPPED:
> -case DHCP6_STATE_RS:
>  return 0;
>  }
>
>  if (r >= 0) {
>  log_dhcp6_client(client, "Recv %s",
>   
> dhcp6_message_type_to_string(message->type));
>  }
> @@ -870,15 +867,14 @@ static int client_start(sd_dhcp6_client *client, enum 
> DHCP6State state)
>  sd_event_source_unref(client->timeout_resend_expire);
>  client->timeout_resend = 
> sd_event_source_unref(client->timeout_resend);
>  client->retransmit_time = 0;
>  client->retransmit_count = 0;
>
>  switch (state) {
>  case DHCP6_STATE_STOPPED:
> -case DHCP6_STATE_RS:
>  case DHCP6_STATE_SOLICITATION:
>
>  r = client_ensure_iaid(client);
>  if (r < 0)
>  return r;
>
>  r = dhcp6_network_bind_udp_socket(client->index, NULL);
> --
> 1.9.3
>
>
> ___
> 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 v2] networkd: set route protocol

2014-07-23 Thread Tom Gundersen

On Tue, Jul 22, 2014 at 11:54 PM, Dan Williams  wrote:
> All routes added by networkd are currently set RTPROT_BOOT, which according
> to the kernel means "Route installed during boot" (rtnetlink.h).  But this
> is not always the case as networkd changes routing after boot too.  Since
> the kernel gives more detailed protocols, use them.
>
> With this patch, user-configured static routes now use RTPROT_STATIC (which
> they are) and DHCP routes use RTPROT_DHCP.  There is no define for IPv4LL
> yet, so those are installed as RTPROT_STATIC (though perhaps RTPROT_RA is
> better?).

Nice. Applied.

Thanks!

Tom

> Signed-off-by: Dan Williams 

Btw, we don't do sob, so I dropped this when applying.

> ---
> v2: rebase missed one case of RTPROT_DHCP; now fixed
>
>  src/libsystemd/sd-rtnl/rtnl-message.c |  5 +++--
>  src/libsystemd/sd-rtnl/test-rtnl.c|  2 +-
>  src/network/networkd-link.c   | 14 +++---
>  src/network/networkd-route.c  | 10 +++---
>  src/network/networkd.h|  3 ++-
>  src/systemd/sd-rtnl.h |  2 +-
>  6 files changed, 21 insertions(+), 15 deletions(-)
>
> diff --git a/src/libsystemd/sd-rtnl/rtnl-message.c 
> b/src/libsystemd/sd-rtnl/rtnl-message.c
> index 7f2e398..c50d0ea 100644
> --- a/src/libsystemd/sd-rtnl/rtnl-message.c
> +++ b/src/libsystemd/sd-rtnl/rtnl-message.c
> @@ -129,15 +129,16 @@ int sd_rtnl_message_route_set_scope(sd_rtnl_message *m, 
> unsigned char scope) {
>
>  rtm->rtm_scope = scope;
>
>  return 0;
>  }
>
>  int sd_rtnl_message_new_route(sd_rtnl *rtnl, sd_rtnl_message **ret,
> -  uint16_t nlmsg_type, int rtm_family) {
> +  uint16_t nlmsg_type, int rtm_family,
> +  unsigned char rtm_protocol) {
>  struct rtmsg *rtm;
>  int r;
>
>  assert_return(rtnl_message_type_is_route(nlmsg_type), -EINVAL);
>  assert_return(rtm_family == AF_INET || rtm_family == AF_INET6, 
> -EINVAL);
>  assert_return(ret, -EINVAL);
>
> @@ -150,15 +151,15 @@ int sd_rtnl_message_new_route(sd_rtnl *rtnl, 
> sd_rtnl_message **ret,
>
>  rtm = NLMSG_DATA((*ret)->hdr);
>
>  rtm->rtm_family = rtm_family;
>  rtm->rtm_scope = RT_SCOPE_UNIVERSE;
>  rtm->rtm_type = RTN_UNICAST;
>  rtm->rtm_table = RT_TABLE_MAIN;
> -rtm->rtm_protocol = RTPROT_BOOT;
> +rtm->rtm_protocol = rtm_protocol;
>
>  return 0;
>  }
>
>  int sd_rtnl_message_link_set_flags(sd_rtnl_message *m, unsigned flags, 
> unsigned change) {
>  struct ifinfomsg *ifi;
>
> diff --git a/src/libsystemd/sd-rtnl/test-rtnl.c 
> b/src/libsystemd/sd-rtnl/test-rtnl.c
> index 082c9e4..4b6e533 100644
> --- a/src/libsystemd/sd-rtnl/test-rtnl.c
> +++ b/src/libsystemd/sd-rtnl/test-rtnl.c
> @@ -128,15 +128,15 @@ static void test_address_get(sd_rtnl *rtnl, int 
> ifindex) {
>
>  static void test_route(void) {
>  _cleanup_rtnl_message_unref_ sd_rtnl_message *req;
>  struct in_addr addr, addr_data;
>  uint32_t index = 2, u32_data;
>  int r;
>
> -r = sd_rtnl_message_new_route(NULL, &req, RTM_NEWROUTE, AF_INET);
> +r = sd_rtnl_message_new_route(NULL, &req, RTM_NEWROUTE, AF_INET, 
> RTPROT_STATIC);
>  if (r < 0) {
>  log_error("Could not create RTM_NEWROUTE message: %s", 
> strerror(-r));
>  return;
>  }
>
>  addr.s_addr = htonl(INADDR_LOOPBACK);
>
> diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c
> index 0a6f524..5b70578 100644
> --- a/src/network/networkd-link.c
> +++ b/src/network/networkd-link.c
> @@ -411,15 +411,15 @@ static int link_set_dhcp_routes(Link *link) {
>  log_warning_link(link, "DHCP error: could not get 
> routes: %s", strerror(-n));
>  return n;
>  }
>
>  for (i = 0; i < n; i++) {
>  _cleanup_route_free_ Route *route = NULL;
>
> -r = route_new_dynamic(&route);
> +r = route_new_dynamic(&route, RTPROT_DHCP);
>  if (r < 0) {
>  log_error_link(link, "Could not allocate route: %s",
> strerror(-r));
>  return r;
>  }
>
>  route->family = AF_INET;
> @@ -477,15 +477,15 @@ static int link_enter_set_routes(Link *link) {
>  if (r < 0 && r != -ENOENT) {
>  log_warning_link(link, "IPV4LL error: no address: 
> %s",
>  strerror(-r));
>  return r;
>  }
>
>  if (r != -ENOENT) {
> -r = route_new_dynamic(&route);
> +r = route_new_dynamic(&route, RTPROT_STATIC);
>  if (r < 0) {
>  log_error_link(link, "Could not allocate 
>

52 matches

Mail list logo