All of lore.kernel.org
 help / color / mirror / Atom feed
From: Petr Vorel <pvorel@suse.cz>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH 1/2] lib: Add safe timerfd macros
Date: Wed, 4 Mar 2020 17:28:57 +0100	[thread overview]
Message-ID: <20200304162857.GA30492@dell5510> (raw)
In-Reply-To: <b42e93cb-b06d-538c-a5ff-1d110ed8ce74@suse.cz>

Hi Martin,

> Please split the following code off to a separate .c file:
OK, I'll do :). We had some discussions about it, I remember problems caused by
using lapi files in the library headers (tst_device.h), I'd consider ok having
just header tst_safe_timerfd.h unless it's loaded by library itself, but
somebody could do it later. And I also consider it as a cleaner solution
(hoping that linker will really drop unused symbols).

> > +#include <errno.h>
> > +#include "lapi/timerfd.h"
> > +#include "tst_test.h"
> > +
> > +#define TTYPE (errno == ENOTSUP ? TCONF : TBROK)
> > +
> > +static inline int safe_timerfd_create(const char *file, const int lineno,
> > +				      int clockid, int flags)
> > +{
> > +	int fd;
> > +

> Don't forget to clear errno when you're not using the TEST() macro.
Do you think it's necessary? I've noticed you clear errno here in original test,
but I'd expect errno is be set if (fd < 0).
But ok, it's done in some macros (tst_safe_macros.c, safe_macros.c)

> > +	if (tst_kvercmp(2, 6, 26) <= 0)
> > +		flags = 0;

> I think tst_brk(TCONF) would be better here. Or at least tst_res(TWARN),
> since resetting flags to 0 may render some tests useless.
Well, this is just to keep test working under 2.6.25 and .26. Obviously it
would not affect many users, but these would got broken these tests
timerfd01.c (all)
timerfd_settime01.c (3th and 4rd test)
timerfd_settime02.c (not sure whether this old version is really affected by
CVE-2017-10661)

break all tests). Agree that hiding it is not good.

> > +
> > +	rval = timerfd_settime(fd, flags, new_value, old_value);
> > +	if (rval < 0) {
> > +		tst_brk(TTYPE | TERRNO, "%s:%d: timerfd_settime() failed",
> > +			file, lineno);
> > +	}
> > +
> > +	return rval;
> > +}

> Split off up to here.
I guess you mean that macros should be kept in header (sure).

> > +
> > +#define SAFE_TIMERFD_CREATE(clockid, flags)\
> > +	safe_timerfd_create(__FILE__, __LINE__, (clockid), (flags))
> > +
> > +#define SAFE_TIMERFD_GETTIME(fd, curr_value)\
> > +	safe_timerfd_gettime(__FILE__, __LINE__, (fd), (curr_value))
> > +
> > +#define SAFE_TIMERFD_SETTIME(fd, flags, new_value, old_value)\
> > +	safe_timerfd_settime(__FILE__, __LINE__, (fd), (flags), (new_value), \
> > +						 (old_value))
> > +
> > +#endif /* SAFE_TIMERFD_H__ */

Kind regards,
Petr

  parent reply	other threads:[~2020-03-04 16:28 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-04 15:12 [LTP] [PATCH 1/2] lib: Add safe timerfd macros Petr Vorel
2020-03-04 15:12 ` [LTP] [PATCH 2/2] timerfd: Use safe macros Petr Vorel
2020-03-04 16:03   ` Martin Doucha
2020-03-04 15:22 ` [LTP] [PATCH 1/2] lib: Add safe timerfd macros Cyril Hrubis
2020-03-04 15:56 ` Martin Doucha
2020-03-04 16:02   ` Martin Doucha
2020-03-04 16:45     ` Petr Vorel
2020-03-05 10:11       ` Cyril Hrubis
2020-03-05 11:54         ` Petr Vorel
2020-03-05 12:21           ` Petr Vorel
2020-03-04 16:03   ` Cyril Hrubis
2020-03-04 16:04     ` Cyril Hrubis
2020-03-04 16:49     ` Petr Vorel
2020-03-04 16:28   ` Petr Vorel [this message]
  -- strict thread matches above, loose matches on Subject: below --
2020-03-04 18:38 Petr Vorel
2020-03-04 18:49 ` Petr Vorel
2020-03-05 10:28 ` Cyril Hrubis

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200304162857.GA30492@dell5510 \
    --to=pvorel@suse.cz \
    --cc=ltp@lists.linux.it \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.