netdev - Re: [PATCH v1 net 02/11] selftest: af_unix: Add msg

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240626014555.86837-1-kuniyu@amazon.com>
Date: Tue, 25 Jun 2024 18:45:55 -0700
From: Kuniyuki Iwashima <kuniyu@...zon.com>
To: <kuba@...nel.org>
CC: <Rao.Shoaib@...cle.com>, <davem@...emloft.net>, <edumazet@...gle.com>,
	<kuni1840@...il.com>, <kuniyu@...zon.com>, <netdev@...r.kernel.org>,
	<pabeni@...hat.com>
Subject: Re: [PATCH v1 net 02/11] selftest: af_unix: Add msg_oob.c.

From: Jakub Kicinski <kuba@...nel.org>
Date: Tue, 25 Jun 2024 17:44:49 -0700
> On Mon, 24 Jun 2024 18:36:36 -0700 Kuniyuki Iwashima wrote:
> > +	if (ret[0] != expected_len || recv_errno[0] != expected_errno) {
> > +		TH_LOG("AF_UNIX :%s", ret[0] < 0 ? strerror(recv_errno[0]) : recv_buf[0]);
> > +		TH_LOG("Expected:%s", expected_errno ? strerror(expected_errno) : expected_buf);
> > +
> > +		ASSERT_EQ(ret[0], expected_len);
> > +		ASSERT_EQ(recv_errno[0], expected_errno);
> > +	}
> 
> repeating the conditions feels slightly imperfect.

Yeah actually I don't like this...

> Would it be possible to modify EXPECT_* to return the condition?
> Then we could:
> 
> 	if (EXPECT(...)) {
> 		TH_LOG(...
> 		TH_LOG(...
> 	}

We can use EXPECT_EQ() {} here, but for some test cases where TCP is
buggy, I'd like to print the difference but let the test pass.

For example, see patch 6.

  #  RUN           msg_oob.no_peek.ex_oob_ahead_break ...
  # msg_oob.c:146:ex_oob_ahead_break:AF_UNIX :hellowol
  # msg_oob.c:147:ex_oob_ahead_break:TCP     :helloworl
                                                     ^
            TCP recv()s already recv()ed data, "r" --'

  #            OK  msg_oob.no_peek.ex_oob_ahead_break
  ok 11 msg_oob.no_peek.ex_oob_ahead_break

In this case, this does not print the recv()ed data,

  if (self->tcp_compliant) {
      EXPECT_EQ(...) {
          /* log retval, errno, buffer */
      }
  }

and this fails the test even though AF_UNIX is doing correct.

  EXPECT_EQ(...) {
      if (self->tcp_compliant) {
          /* log retval, errno, buffer */
      }
  }

I think we can convert it to EXPECT_EQ() {} in all places after
fixing TCP side and removing tcp_incompliant{} uses in the test.