* [PATCH] passt-repair: Don't use perror(), accept ECONNRESET as termination
@ 2025-02-07 0:54 Stefano Brivio
2025-02-07 1:15 ` David Gibson
0 siblings, 1 reply; 5+ messages in thread
From: Stefano Brivio @ 2025-02-07 0:54 UTC (permalink / raw)
To: passt-dev
If we use glibc's perror(), we need to allow dup() and fcntl() in our
seccomp profiles, which are a bit too much for this simple helper. On
top of that, we would probably need a wrapper to avoid allocation for
translated messages.
While at it: ECONNRESET is just a close() from passt, treat it like
EOF.
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
passt-repair.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/passt-repair.c b/passt-repair.c
index 3c3247b..d137a18 100644
--- a/passt-repair.c
+++ b/passt-repair.c
@@ -95,7 +95,7 @@ int main(int argc, char **argv)
}
if ((s = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) {
- perror("Failed to create AF_UNIX socket");
+ fprintf(stderr, "Failed to create AF_UNIX socket: %i\n", errno);
_exit(1);
}
@@ -108,8 +108,12 @@ int main(int argc, char **argv)
loop:
ret = recvmsg(s, &msg, 0);
if (ret < 0) {
- perror("Failed to receive message");
- _exit(1);
+ if (errno == ECONNRESET) {
+ ret = 0;
+ } else {
+ fprintf(stderr, "Failed to read message: %i\n", errno);
+ _exit(1);
+ }
}
if (!ret) /* Done */
--
@@ -95,7 +95,7 @@ int main(int argc, char **argv)
}
if ((s = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) {
- perror("Failed to create AF_UNIX socket");
+ fprintf(stderr, "Failed to create AF_UNIX socket: %i\n", errno);
_exit(1);
}
@@ -108,8 +108,12 @@ int main(int argc, char **argv)
loop:
ret = recvmsg(s, &msg, 0);
if (ret < 0) {
- perror("Failed to receive message");
- _exit(1);
+ if (errno == ECONNRESET) {
+ ret = 0;
+ } else {
+ fprintf(stderr, "Failed to read message: %i\n", errno);
+ _exit(1);
+ }
}
if (!ret) /* Done */
--
2.43.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] passt-repair: Don't use perror(), accept ECONNRESET as termination
2025-02-07 0:54 [PATCH] passt-repair: Don't use perror(), accept ECONNRESET as termination Stefano Brivio
@ 2025-02-07 1:15 ` David Gibson
2025-02-07 1:29 ` Stefano Brivio
0 siblings, 1 reply; 5+ messages in thread
From: David Gibson @ 2025-02-07 1:15 UTC (permalink / raw)
To: Stefano Brivio; +Cc: passt-dev
[-- Attachment #1: Type: text/plain, Size: 1621 bytes --]
On Fri, Feb 07, 2025 at 01:54:39AM +0100, Stefano Brivio wrote:
> If we use glibc's perror(), we need to allow dup() and fcntl() in our
> seccomp profiles, which are a bit too much for this simple helper. On
> top of that, we would probably need a wrapper to avoid allocation for
> translated messages.
>
> While at it: ECONNRESET is just a close() from passt, treat it like
> EOF.
>
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> passt-repair.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/passt-repair.c b/passt-repair.c
> index 3c3247b..d137a18 100644
> --- a/passt-repair.c
> +++ b/passt-repair.c
> @@ -95,7 +95,7 @@ int main(int argc, char **argv)
> }
>
> if ((s = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) {
> - perror("Failed to create AF_UNIX socket");
> + fprintf(stderr, "Failed to create AF_UNIX socket:
> %i\n", errno);
We could use strerror_() here, couldn't we?
> _exit(1);
> }
>
> @@ -108,8 +108,12 @@ int main(int argc, char **argv)
> loop:
> ret = recvmsg(s, &msg, 0);
> if (ret < 0) {
> - perror("Failed to receive message");
> - _exit(1);
> + if (errno == ECONNRESET) {
> + ret = 0;
> + } else {
> + fprintf(stderr, "Failed to read message: %i\n", errno);
> + _exit(1);
> + }
> }
>
> if (!ret) /* Done */
--
David Gibson (he or they) | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you, not the other way
| around.
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] passt-repair: Don't use perror(), accept ECONNRESET as termination
2025-02-07 1:15 ` David Gibson
@ 2025-02-07 1:29 ` Stefano Brivio
2025-02-07 2:38 ` David Gibson
0 siblings, 1 reply; 5+ messages in thread
From: Stefano Brivio @ 2025-02-07 1:29 UTC (permalink / raw)
To: David Gibson; +Cc: passt-dev
On Fri, 7 Feb 2025 12:15:35 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:
> On Fri, Feb 07, 2025 at 01:54:39AM +0100, Stefano Brivio wrote:
> > If we use glibc's perror(), we need to allow dup() and fcntl() in our
> > seccomp profiles, which are a bit too much for this simple helper. On
> > top of that, we would probably need a wrapper to avoid allocation for
> > translated messages.
> >
> > While at it: ECONNRESET is just a close() from passt, treat it like
> > EOF.
> >
> > Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
>
> > ---
> > passt-repair.c | 10 +++++++---
> > 1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/passt-repair.c b/passt-repair.c
> > index 3c3247b..d137a18 100644
> > --- a/passt-repair.c
> > +++ b/passt-repair.c
> > @@ -95,7 +95,7 @@ int main(int argc, char **argv)
> > }
> >
> > if ((s = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) {
> > - perror("Failed to create AF_UNIX socket");
> > + fprintf(stderr, "Failed to create AF_UNIX socket:
> > %i\n", errno);
>
> We could use strerror_() here, couldn't we?
We would need to link that, which is quite some code (the whole
strerrordesc_np())... I mean, this runs privileged.
--
Stefano
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] passt-repair: Don't use perror(), accept ECONNRESET as termination
2025-02-07 1:29 ` Stefano Brivio
@ 2025-02-07 2:38 ` David Gibson
2025-02-07 9:42 ` Stefano Brivio
0 siblings, 1 reply; 5+ messages in thread
From: David Gibson @ 2025-02-07 2:38 UTC (permalink / raw)
To: Stefano Brivio; +Cc: passt-dev
[-- Attachment #1: Type: text/plain, Size: 1822 bytes --]
On Fri, Feb 07, 2025 at 02:29:23AM +0100, Stefano Brivio wrote:
> On Fri, 7 Feb 2025 12:15:35 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
>
> > On Fri, Feb 07, 2025 at 01:54:39AM +0100, Stefano Brivio wrote:
> > > If we use glibc's perror(), we need to allow dup() and fcntl() in our
> > > seccomp profiles, which are a bit too much for this simple helper. On
> > > top of that, we would probably need a wrapper to avoid allocation for
> > > translated messages.
> > >
> > > While at it: ECONNRESET is just a close() from passt, treat it like
> > > EOF.
> > >
> > > Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> >
> > Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> >
> > > ---
> > > passt-repair.c | 10 +++++++---
> > > 1 file changed, 7 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/passt-repair.c b/passt-repair.c
> > > index 3c3247b..d137a18 100644
> > > --- a/passt-repair.c
> > > +++ b/passt-repair.c
> > > @@ -95,7 +95,7 @@ int main(int argc, char **argv)
> > > }
> > >
> > > if ((s = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) {
> > > - perror("Failed to create AF_UNIX socket");
> > > + fprintf(stderr, "Failed to create AF_UNIX socket:
> > > %i\n", errno);
> >
> > We could use strerror_() here, couldn't we?
>
> We would need to link that, which is quite some code (the whole
> strerrordesc_np())... I mean, this runs privileged.
Hrm, is the non-locale one really that much code though? Shouldn't it
be about 5 lines of code and a table of strings? But then.. glibc is
really good at making things complicated.
--
David Gibson (he or they) | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you, not the other way
| around.
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] passt-repair: Don't use perror(), accept ECONNRESET as termination
2025-02-07 2:38 ` David Gibson
@ 2025-02-07 9:42 ` Stefano Brivio
0 siblings, 0 replies; 5+ messages in thread
From: Stefano Brivio @ 2025-02-07 9:42 UTC (permalink / raw)
To: David Gibson; +Cc: passt-dev
On Fri, 7 Feb 2025 13:38:10 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:
> On Fri, Feb 07, 2025 at 02:29:23AM +0100, Stefano Brivio wrote:
> > On Fri, 7 Feb 2025 12:15:35 +1100
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> >
> > > On Fri, Feb 07, 2025 at 01:54:39AM +0100, Stefano Brivio wrote:
> > > > If we use glibc's perror(), we need to allow dup() and fcntl() in our
> > > > seccomp profiles, which are a bit too much for this simple helper. On
> > > > top of that, we would probably need a wrapper to avoid allocation for
> > > > translated messages.
> > > >
> > > > While at it: ECONNRESET is just a close() from passt, treat it like
> > > > EOF.
> > > >
> > > > Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> > >
> > > Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> > >
> > > > ---
> > > > passt-repair.c | 10 +++++++---
> > > > 1 file changed, 7 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/passt-repair.c b/passt-repair.c
> > > > index 3c3247b..d137a18 100644
> > > > --- a/passt-repair.c
> > > > +++ b/passt-repair.c
> > > > @@ -95,7 +95,7 @@ int main(int argc, char **argv)
> > > > }
> > > >
> > > > if ((s = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) {
> > > > - perror("Failed to create AF_UNIX socket");
> > > > + fprintf(stderr, "Failed to create AF_UNIX socket:
> > > > %i\n", errno);
> > >
> > > We could use strerror_() here, couldn't we?
> >
> > We would need to link that, which is quite some code (the whole
> > strerrordesc_np())... I mean, this runs privileged.
>
> Hrm, is the non-locale one really that much code though? Shouldn't it
> be about 5 lines of code and a table of strings? But then.. glibc is
> really good at making things complicated.
Right, and the "table of strings" is probably not great to have here.
But the lines of code I was referring to are the weak aliasing trick
(musl doesn't have strerrordesc_np(), which is not POSIX). I could
include util.h, but that means including even more code...
Really, I think numbers fit error messages printed by a privileged
component pretty well.
--
Stefano
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-02-07 9:42 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-02-07 0:54 [PATCH] passt-repair: Don't use perror(), accept ECONNRESET as termination Stefano Brivio
2025-02-07 1:15 ` David Gibson
2025-02-07 1:29 ` Stefano Brivio
2025-02-07 2:38 ` David Gibson
2025-02-07 9:42 ` Stefano Brivio
Code repositories for project(s) associated with this public inbox
https://passt.top/passt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).