public inbox for ltp@lists.linux.it
 help / color / mirror / Atom feed
From: Eric Biggers <ebiggers3@gmail.com>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH] syscalls/keyctl06: Print TFAIL if keyring_read() returns wrong size
Date: Wed, 18 Oct 2017 10:19:12 -0700	[thread overview]
Message-ID: <20171018171912.GA126485@gmail.com> (raw)
In-Reply-To: <59E6F3D5.8040001@cn.fujitsu.com>

On Wed, Oct 18, 2017 at 02:25:25PM +0800, Xiao Yang wrote:
> >It was actually pointed out yesterday that the short return value is a bug in
> >the kernel patch.  The documented behavior of keyctl_read() (as well as the
> >actual behavior for the other key types that implement it) is to return the full
> >count on a short read, rather than a short count.  It's not really intuitive but
> >I'm going to have to fix it with another kernel patch.
> Thanks for your explanation.
> Sorry, i misunderstood the expected return value before.
> 
> 
> >For now we probably should just make the test accept both return values:
> >
> >	if (TEST_RETURN != sizeof(key_serial_t)&&
> >	TEST_RETURN != sizeof(key_ids)) {
> >		tst_brk(TBROK, "KEYCTL_READ returned %ld but expected %zu or %zu",
> >			TEST_RETURN, sizeof(key_serial_t), sizeof(key_ids));
> >	}
> >
> >Then once there is another kernel patch, I'll update the test to reference that
> >commit too, and accept only TEST_RETURN == sizeof(key_ids).
> Could we update the test to check both return values? as below:
> if (TEST_RETURN != sizeof(key_ids)) {
> 	/* keyctl_read() should return the size of buffer required, rather than the size
> 	 * of data read into buffer. This bug was introduced by the commit:
> 	 * e645016abc80 ("KEYS: fix writing past end of user-supplied buffer in keyring_read()")
> 	 */
> 	if (TEST_RETURN == sizeof(key_serial_t)) {
> 		tst_brk(TFAIL, "KEYCTL_READ returned %ld but expected %zu",
> 			TEST_RETURN, sizeof(key_ids));
> 	}
> 
> 	tst_brk(TBROK, "KEYCTL_READ returned %ld but expected %zu",
> 		TEST_RETURN, sizeof(key_ids));
> }
> 
> We probably should expose the short return value as a bug, rather than ignore it.
> 

This will confuse people when the comment at the top of the file says
"regression test for commit e645016abc80", so people will waste time trying to
figure out why the test is still failing.  I'd prefer to wait to require the
full return value until I get a second fix applied, which can then be explicitly
referenced from the test.

Eric

  reply	other threads:[~2017-10-18 17:19 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-17 12:53 [LTP] [PATCH] syscalls/keyctl06: Print TFAIL if keyring_read() returns wrong size Xiao Yang
2017-10-17 16:12 ` Eric Biggers
2017-10-18  6:25   ` Xiao Yang
2017-10-18 17:19     ` Eric Biggers [this message]
2017-10-19  2:14       ` Xiao Yang
2017-10-19  6:09       ` [LTP] [PATCH v2] syscalls/keyctl06: Accept two kinds of return values for the time being Xiao Yang
2017-10-27  9:00       ` [LTP] [PATCH v3] syscalls/keyctl06: Fix return value Xiao Yang

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=20171018171912.GA126485@gmail.com \
    --to=ebiggers3@gmail.com \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox