* Re: [PATCH v4 3/3] fs: Add aio iopriority support for block_dev
From: kbuild test robot @ 2018-05-18 20:08 UTC (permalink / raw)
Cc: kbuild-all, viro, linux-fsdevel, axboe, bcrl, mingo, peterz,
pombredanne, gregkh, bigeasy, rgoldwyn, linux-block, linux-kernel,
linux-aio, linux-api, Adam Manzanares
In-Reply-To: <20180517203803.2664-4-adam.manzanares@wdc.com>
[-- Attachment #1: Type: text/plain, Size: 2322 bytes --]
Hi Adam,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on next-20180516]
[cannot apply to linus/master block/for-next v4.17-rc5 v4.17-rc4 v4.17-rc3 v4.17-rc5]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/adam-manzanares-wdc-com/AIO-add-per-command-iopriority/20180519-031848
config: x86_64-randconfig-x013-201819 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
fs/aio.c: In function 'aio_prep_rw':
>> fs/aio.c:1460:9: error: implicit declaration of function 'ioprio_check_cap'; did you mean 'param_check_charp'? [-Werror=implicit-function-declaration]
ret = ioprio_check_cap(iocb->aio_reqprio);
^~~~~~~~~~~~~~~~
param_check_charp
cc1: some warnings being treated as errors
vim +1460 fs/aio.c
1440
1441 static int aio_prep_rw(struct kiocb *req, struct iocb *iocb)
1442 {
1443 int ret;
1444
1445 req->ki_filp = fget(iocb->aio_fildes);
1446 if (unlikely(!req->ki_filp))
1447 return -EBADF;
1448 req->ki_complete = aio_complete_rw;
1449 req->ki_pos = iocb->aio_offset;
1450 req->ki_flags = iocb_flags(req->ki_filp);
1451 if (iocb->aio_flags & IOCB_FLAG_RESFD)
1452 req->ki_flags |= IOCB_EVENTFD;
1453 req->ki_hint = file_write_hint(req->ki_filp);
1454 if (iocb->aio_flags & IOCB_FLAG_IOPRIO) {
1455 /*
1456 * If the IOCB_FLAG_IOPRIO flag of aio_flags is set, then
1457 * aio_reqprio is interpreted as an I/O scheduling
1458 * class and priority.
1459 */
> 1460 ret = ioprio_check_cap(iocb->aio_reqprio);
1461 if (ret) {
1462 pr_debug("aio ioprio check cap error\n");
1463 return -EINVAL;
1464 }
1465
1466 req->ki_ioprio = iocb->aio_reqprio;
1467 req->ki_flags |= IOCB_IOPRIO;
1468 }
1469
1470 ret = kiocb_set_rw_flags(req, iocb->aio_rw_flags);
1471 if (unlikely(ret))
1472 fput(req->ki_filp);
1473 return ret;
1474 }
1475
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 24001 bytes --]
^ permalink raw reply
* Re: [PATCH v4 3/3] fs: Add aio iopriority support for block_dev
From: Adam Manzanares @ 2018-05-18 19:54 UTC (permalink / raw)
To: Christoph Hellwig
Cc: viro@zeniv.linux.org.uk, linux-fsdevel@vger.kernel.org,
axboe@kernel.dk, bcrl@kvack.org, mingo@kernel.org,
peterz@infradead.org, pombredanne@nexb.com,
gregkh@linuxfoundation.org, bigeasy@linutronix.de,
rgoldwyn@suse.com, linux-block@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-aio@kvack.org,
linux-api@vger.kernel.org
In-Reply-To: <20180518160636.GH22791@infradead.org>
On 5/18/18 9:06 AM, Christoph Hellwig wrote:
> Looks fine, although I'd split it into a aio and block_dev patch.
>
> Also please wire this up for the fs/iomap.c direct I/O code, it should
> be essentially the same sniplet as in the block_dev.c code.
>
Will do.
^ permalink raw reply
* Re: [PATCH v4 2/3] fs: Convert kiocb rw_hint from enum to u16
From: Adam Manzanares @ 2018-05-18 19:53 UTC (permalink / raw)
To: Christoph Hellwig
Cc: viro@zeniv.linux.org.uk, linux-fsdevel@vger.kernel.org,
axboe@kernel.dk, bcrl@kvack.org, mingo@kernel.org,
peterz@infradead.org, pombredanne@nexb.com,
gregkh@linuxfoundation.org, bigeasy@linutronix.de,
rgoldwyn@suse.com, linux-block@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-aio@kvack.org,
linux-api@vger.kernel.org
In-Reply-To: <20180518160512.GG22791@infradead.org>
On 5/18/18 9:05 AM, Christoph Hellwig wrote:
>> +/* ki_hint changed from enum to u16, make sure rw_hint fits into u16 */
>
> I don't think this comment is very useful.
>
>> +static inline u16 ki_hint_valid(enum rw_hint hint)
>
> I'd call this ki_hint_validate.
>
>> +{
>> + if (hint > MAX_KI_HINT)
>> + return 0;
>> +
>> + return hint;
>
> Nit: kill the empty line.
>
I'll clean this up in the next revision.
^ permalink raw reply
* Re: [PATCH 07/14] powerpc: Add support for restartable sequences
From: Mathieu Desnoyers @ 2018-05-18 18:17 UTC (permalink / raw)
To: Boqun Feng
Cc: Will Deacon, Peter Zijlstra, Paul E. McKenney, Andy Lutomirski,
Dave Watson, linux-kernel, linux-api, Paul Turner, Andrew Morton,
Russell King, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
Andrew Hunter, Andi Kleen, Chris Lameter, Ben Maurer, rostedt,
Josh Triplett, Linus Torvalds, Catalin Marinas <cata>
In-Reply-To: <1526601043.1338308.1376191416.0444B8C5@webmail.messagingengine.com>
----- On May 17, 2018, at 7:50 PM, Boqun Feng boqun.feng@gmail.com wrote:
[...]
>> > I think you're right. So we have to introduce callsite to rseq_syscall()
>> > in syscall path, something like:
>> >
>> > diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
>> > index 51695608c68b..a25734a96640 100644
>> > --- a/arch/powerpc/kernel/entry_64.S
>> > +++ b/arch/powerpc/kernel/entry_64.S
>> > @@ -222,6 +222,9 @@ system_call_exit:
>> > mtmsrd r11,1
>> > #endif /* CONFIG_PPC_BOOK3E */
>> >
>> > + addi r3,r1,STACK_FRAME_OVERHEAD
>> > + bl rseq_syscall
>> > +
>> > ld r9,TI_FLAGS(r12)
>> > li r11,-MAX_ERRNO
>> > andi.
>> > r0,r9,(_TIF_SYSCALL_DOTRACE|_TIF_SINGLESTEP|_TIF_USER_WORK_MASK|_TIF_PERSYSCALL_MASK)
>> >
By the way, I think this is not the right spot to call rseq_syscall, because
interrupts are disabled. I think we should move this hunk right after system_call_exit.
Would you like to implement and test an updated patch adding those calls for ppc 32 and 64 ?
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [PATCH v4 3/3] fs: Add aio iopriority support for block_dev
From: Christoph Hellwig @ 2018-05-18 16:06 UTC (permalink / raw)
To: adam.manzanares
Cc: viro, linux-fsdevel, axboe, bcrl, mingo, peterz, pombredanne,
gregkh, bigeasy, rgoldwyn, linux-block, linux-kernel, linux-aio,
linux-api
In-Reply-To: <20180517203803.2664-4-adam.manzanares@wdc.com>
Looks fine, although I'd split it into a aio and block_dev patch.
Also please wire this up for the fs/iomap.c direct I/O code, it should
be essentially the same sniplet as in the block_dev.c code.
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH v4 2/3] fs: Convert kiocb rw_hint from enum to u16
From: Christoph Hellwig @ 2018-05-18 16:05 UTC (permalink / raw)
To: adam.manzanares
Cc: viro, linux-fsdevel, axboe, bcrl, mingo, peterz, pombredanne,
gregkh, bigeasy, rgoldwyn, linux-block, linux-kernel, linux-aio,
linux-api
In-Reply-To: <20180517203803.2664-3-adam.manzanares@wdc.com>
> +/* ki_hint changed from enum to u16, make sure rw_hint fits into u16 */
I don't think this comment is very useful.
> +static inline u16 ki_hint_valid(enum rw_hint hint)
I'd call this ki_hint_validate.
> +{
> + if (hint > MAX_KI_HINT)
> + return 0;
> +
> + return hint;
Nit: kill the empty line.
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH v4 1/3] block: add ioprio_check_cap function
From: Christoph Hellwig @ 2018-05-18 16:04 UTC (permalink / raw)
To: adam.manzanares
Cc: viro, linux-fsdevel, axboe, bcrl, mingo, peterz, pombredanne,
gregkh, bigeasy, rgoldwyn, linux-block, linux-kernel, linux-aio,
linux-api
In-Reply-To: <20180517203803.2664-2-adam.manzanares@wdc.com>
On Thu, May 17, 2018 at 01:38:01PM -0700, adam.manzanares@wdc.com wrote:
> From: Adam Manzanares <adam.manzanares@wdc.com>
>
> Aio per command iopriority support introduces a second interface between
> userland and the kernel capable of passing iopriority. The aio interface also
> needs the ability to verify that the submitting context has sufficient
> priviledges to submit IOPRIO_RT commands. This patch creates the
> ioprio_check_cap function to be used by the ioprio_set system call and also by
> the aio interface.
>
> Signed-off-by: Adam Manzanares <adam.manzanares@wdc.com>
Looks fine,
Reviewed-by: Christoph Hellwig <hch@lst.de>
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [RFC PATCH ghak32 V2 01/13] audit: add container id
From: Steve Grubb @ 2018-05-18 15:38 UTC (permalink / raw)
To: Richard Guy Briggs
Cc: cgroups, containers, linux-api, Linux-Audit Mailing List,
linux-fsdevel, LKML, netdev, ebiederm, luto, jlayton, carlos,
dhowells, viro, simo, eparis, serge
In-Reply-To: <20180518152106.do5b3mu6e6eyvo7q@madcap2.tricolour.ca>
On Fri, 18 May 2018 11:21:06 -0400
Richard Guy Briggs <rgb@redhat.com> wrote:
> On 2018-05-18 09:56, Steve Grubb wrote:
> > On Thu, 17 May 2018 17:56:00 -0400
> > Richard Guy Briggs <rgb@redhat.com> wrote:
> >
> > > > During syscall events, the path info is returned in a a record
> > > > simply called AUDIT_PATH, cwd info is returned in AUDIT_CWD. So,
> > > > rather than calling the record that gets attached to everything
> > > > AUDIT_CONTAINER_INFO, how about simply AUDIT_CONTAINER.
> > >
> > > Considering the container initiation record is different than the
> > > record to document the container involved in an otherwise normal
> > > syscall, we need two names. I don't have a strong opinion what
> > > they are.
> > >
> > > I'd prefer AUDIT_CONTAIN and AUDIT_CONTAINER_INFO so that the two
> > > are different enough to be visually distinct while leaving
> > > AUDIT_CONTAINERID for the field type in patch 4 ("audit: add
> > > containerid filtering")
>
> (Sorry, I had intended AUDIT_CONTAINER for the first in that paragraph
> above.)
>
> > How about AUDIT_CONTAINER for the auxiliary record? The one that
> > starts the container, I don't have a strong opinion on. Could be
> > AUDIT_CONTAINER_INIT, AUDIT_CONTAINER_START, AUDIT_CONTAINERID,
> > AUDIT_CONTAINER_ID, or something else. The API call that sets the ID
> > for filtering could be AUDIT_CID or AUDIT_CONTID if that helps
> > decide what the initial event might be. Normally, it should match
> > the field being filtered.
>
> Ok, I had shortened the record field name to "contid=" to be unique
> enough while not using too much netlink bandwidth. I could have used
> "cid=" but that could be unobvious or ambiguous. I didn't want to use
> the full "containerid=" due to that. I suppose I could change the
> field name macro to AUDIT_CONTID.
>
> For the one that starts the container, I'd prefer to leave the name a
> bit more general than "_INIT", "_START", so maybe I'll swap them
> around and use AUDIT_CONTAINER_INFO for the startup record, and use
> AUDIT_CONTAINER for the syscall auxiliary record.
>
> Does that work?
I'll go along with that. Thanks. But making that swap frees up
AUDIT_CONTAINER_ID which could be the first event. But
AUDIT_CONTAINER_INFO is also fine with me.
Best Regards,
-Steve
^ permalink raw reply
* Re: [PATCH v4 3/3] fs: Add aio iopriority support for block_dev
From: Adam Manzanares @ 2018-05-18 15:29 UTC (permalink / raw)
To: Jens Axboe, viro@zeniv.linux.org.uk,
linux-fsdevel@vger.kernel.org, bcrl@kvack.org
Cc: mingo@kernel.org, peterz@infradead.org, pombredanne@nexb.com,
gregkh@linuxfoundation.org, bigeasy@linutronix.de,
rgoldwyn@suse.com, linux-block@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-aio@kvack.org,
linux-api@vger.kernel.org
In-Reply-To: <51cbf2e9-1ff9-b9e7-fb68-055a64d54da5@kernel.dk>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2403 bytes --]
On 5/18/18 8:14 AM, Jens Axboe wrote:
> On 5/17/18 2:38 PM, adam.manzanares@wdc.com wrote:
>> From: Adam Manzanares <adam.manzanares@wdc.com>
>>
>> This is the per-I/O equivalent of the ioprio_set system call.
>>
>> When IOCB_FLAG_IOPRIO is set on the iocb aio_flags field, then we set the
>> newly added kiocb ki_ioprio field to the value in the iocb aio_reqprio field.
>>
>> When a bio is created for an aio request by the block dev we set the priority
>> value of the bio to the user supplied value.
>>
>> This patch depends on block: add ioprio_check_cap function
>
> Actually, one comment on this one:
>
>> diff --git a/fs/aio.c b/fs/aio.c
>> index f3eae5d5771b..ff3107aa82d5 100644
>> --- a/fs/aio.c
>> +++ b/fs/aio.c
>> @@ -1451,6 +1451,22 @@ static int aio_prep_rw(struct kiocb *req, struct iocb *iocb)
>> if (iocb->aio_flags & IOCB_FLAG_RESFD)
>> req->ki_flags |= IOCB_EVENTFD;
>> req->ki_hint = file_write_hint(req->ki_filp);
>> + if (iocb->aio_flags & IOCB_FLAG_IOPRIO) {
>> + /*
>> + * If the IOCB_FLAG_IOPRIO flag of aio_flags is set, then
>> + * aio_reqprio is interpreted as an I/O scheduling
>> + * class and priority.
>> + */
>> + ret = ioprio_check_cap(iocb->aio_reqprio);
>> + if (ret) {
>> + pr_debug("aio ioprio check cap error\n");
>> + return -EINVAL;
>> + }
>> +
>> + req->ki_ioprio = iocb->aio_reqprio;
>> + req->ki_flags |= IOCB_IOPRIO;
>> + }
>
> Do we really need IOCB_IOPRIO? All zeroes is no priority set anyway,
> so we should be able to get by with just setting ->ki_ioprio to either
> the priority, or 0.
>
>> diff --git a/fs/block_dev.c b/fs/block_dev.c
>> index 7ec920e27065..970bef79caa6 100644
>> --- a/fs/block_dev.c
>> +++ b/fs/block_dev.c
>> @@ -355,6 +355,8 @@ __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter, int nr_pages)
>> bio->bi_write_hint = iocb->ki_hint;
>> bio->bi_private = dio;
>> bio->bi_end_io = blkdev_bio_end_io;
>> + if (iocb->ki_flags & IOCB_IOPRIO)
>> + bio->bi_ioprio = iocb->ki_ioprio;
>
> And then this assignment can just happen unconditionally.
That is a cleaner way of guaranteeing the ioprio set on the kiocb is
only set when the user intends to use the ioprio from the iocb.
I'll resend the series.
> N‹§²æìr¸›zǧu©ž²Æ {\b†éì¹»\x1c®&Þ–)îŨ¨Š{ayº\x1dÊÚ&j:+v‰¨’öœ’Šà\x16Šæ¢·¢ú(œ¸§»\x10\b:Çž†Ûiÿü0ÂKÚrJ+ƒö¢£ðèž×¦j)Z†·Ÿ
^ permalink raw reply
* Re: [RFC PATCH ghak32 V2 01/13] audit: add container id
From: Richard Guy Briggs @ 2018-05-18 15:21 UTC (permalink / raw)
To: Steve Grubb
Cc: cgroups, containers, linux-api, Linux-Audit Mailing List,
linux-fsdevel, LKML, netdev, ebiederm, luto, jlayton, carlos,
dhowells, viro, simo, eparis, serge
In-Reply-To: <20180518095636.56ff322d@ivy-bridge>
On 2018-05-18 09:56, Steve Grubb wrote:
> On Thu, 17 May 2018 17:56:00 -0400
> Richard Guy Briggs <rgb@redhat.com> wrote:
>
> > > During syscall events, the path info is returned in a a record
> > > simply called AUDIT_PATH, cwd info is returned in AUDIT_CWD. So,
> > > rather than calling the record that gets attached to everything
> > > AUDIT_CONTAINER_INFO, how about simply AUDIT_CONTAINER.
> >
> > Considering the container initiation record is different than the
> > record to document the container involved in an otherwise normal
> > syscall, we need two names. I don't have a strong opinion what they
> > are.
> >
> > I'd prefer AUDIT_CONTAIN and AUDIT_CONTAINER_INFO so that the two
> > are different enough to be visually distinct while leaving
> > AUDIT_CONTAINERID for the field type in patch 4 ("audit: add
> > containerid filtering")
(Sorry, I had intended AUDIT_CONTAINER for the first in that paragraph
above.)
> How about AUDIT_CONTAINER for the auxiliary record? The one that starts
> the container, I don't have a strong opinion on. Could be
> AUDIT_CONTAINER_INIT, AUDIT_CONTAINER_START, AUDIT_CONTAINERID,
> AUDIT_CONTAINER_ID, or something else. The API call that sets the ID
> for filtering could be AUDIT_CID or AUDIT_CONTID if that helps decide
> what the initial event might be. Normally, it should match the field
> being filtered.
Ok, I had shortened the record field name to "contid=" to be unique
enough while not using too much netlink bandwidth. I could have used
"cid=" but that could be unobvious or ambiguous. I didn't want to use
the full "containerid=" due to that. I suppose I could change the
field name macro to AUDIT_CONTID.
For the one that starts the container, I'd prefer to leave the name a
bit more general than "_INIT", "_START", so maybe I'll swap them around
and use AUDIT_CONTAINER_INFO for the startup record, and use
AUDIT_CONTAINER for the syscall auxiliary record.
Does that work?
> -Steve
- RGB
--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
^ permalink raw reply
* Re: [PATCH v4 3/3] fs: Add aio iopriority support for block_dev
From: Jens Axboe @ 2018-05-18 15:14 UTC (permalink / raw)
To: adam.manzanares, viro, linux-fsdevel, bcrl
Cc: mingo, peterz, pombredanne, gregkh, bigeasy, rgoldwyn,
linux-block, linux-kernel, linux-aio, linux-api
In-Reply-To: <20180517203803.2664-4-adam.manzanares@wdc.com>
On 5/17/18 2:38 PM, adam.manzanares@wdc.com wrote:
> From: Adam Manzanares <adam.manzanares@wdc.com>
>
> This is the per-I/O equivalent of the ioprio_set system call.
>
> When IOCB_FLAG_IOPRIO is set on the iocb aio_flags field, then we set the
> newly added kiocb ki_ioprio field to the value in the iocb aio_reqprio field.
>
> When a bio is created for an aio request by the block dev we set the priority
> value of the bio to the user supplied value.
>
> This patch depends on block: add ioprio_check_cap function
Actually, one comment on this one:
> diff --git a/fs/aio.c b/fs/aio.c
> index f3eae5d5771b..ff3107aa82d5 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -1451,6 +1451,22 @@ static int aio_prep_rw(struct kiocb *req, struct iocb *iocb)
> if (iocb->aio_flags & IOCB_FLAG_RESFD)
> req->ki_flags |= IOCB_EVENTFD;
> req->ki_hint = file_write_hint(req->ki_filp);
> + if (iocb->aio_flags & IOCB_FLAG_IOPRIO) {
> + /*
> + * If the IOCB_FLAG_IOPRIO flag of aio_flags is set, then
> + * aio_reqprio is interpreted as an I/O scheduling
> + * class and priority.
> + */
> + ret = ioprio_check_cap(iocb->aio_reqprio);
> + if (ret) {
> + pr_debug("aio ioprio check cap error\n");
> + return -EINVAL;
> + }
> +
> + req->ki_ioprio = iocb->aio_reqprio;
> + req->ki_flags |= IOCB_IOPRIO;
> + }
Do we really need IOCB_IOPRIO? All zeroes is no priority set anyway,
so we should be able to get by with just setting ->ki_ioprio to either
the priority, or 0.
> diff --git a/fs/block_dev.c b/fs/block_dev.c
> index 7ec920e27065..970bef79caa6 100644
> --- a/fs/block_dev.c
> +++ b/fs/block_dev.c
> @@ -355,6 +355,8 @@ __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter, int nr_pages)
> bio->bi_write_hint = iocb->ki_hint;
> bio->bi_private = dio;
> bio->bi_end_io = blkdev_bio_end_io;
> + if (iocb->ki_flags & IOCB_IOPRIO)
> + bio->bi_ioprio = iocb->ki_ioprio;
And then this assignment can just happen unconditionally.
--
Jens Axboe
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH v4 0/3] AIO add per-command iopriority
From: Adam Manzanares @ 2018-05-18 15:03 UTC (permalink / raw)
To: Jens Axboe, viro@zeniv.linux.org.uk,
linux-fsdevel@vger.kernel.org, bcrl@kvack.org
Cc: mingo@kernel.org, peterz@infradead.org, pombredanne@nexb.com,
gregkh@linuxfoundation.org, bigeasy@linutronix.de,
rgoldwyn@suse.com, linux-block@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-aio@kvack.org,
linux-api@vger.kernel.org
In-Reply-To: <3ec960cf-e14c-6ff6-3356-186136f43df7@kernel.dk>
On 5/17/18 7:41 PM, Jens Axboe wrote:
> On 5/17/18 2:38 PM, adam.manzanares@wdc.com wrote:
>> From: Adam Manzanares <adam.manzanares@wdc.com>
>>
>> This is the per-I/O equivalent of the ioprio_set system call.
>> See the following link for performance implications on a SATA HDD:
>> https://lkml.org/lkml/2016/12/6/495
>>
>> First patch factors ioprio_check_cap function out of ioprio_set system call to
>> also be used by the aio ioprio interface.
>>
>> Second patch converts kiocb ki_hint field to a u16 to avoid kiocb bloat.
>>
>> Third patch passes ioprio hint from aio iocb to kiocb and enables block_dev
>> usage of the per I/O ioprio feature.
>>
>> v2: merge patches
>> use IOCB_FLAG_IOPRIO
>> validate intended use with IOCB_IOPRIO
>> add linux-api and linux-block to cc
>>
>> v3: add ioprio_check_cap function
>> convert kiocb ki_hint to u16
>> use ioprio_check_cap when adding ioprio to kiocb in aio.c
>>
>> v4: handle IOCB_IOPRIO in aio_prep_rw
>> note patch 3 depends on patch 1 in commit msg
>>
>> Adam Manzanares (3):
>> block: add ioprio_check_cap function
>> fs: Convert kiocb rw_hint from enum to u16
>> fs: Add aio iopriority support for block_dev
>>
>> block/ioprio.c | 22 ++++++++++++++++------
>> fs/aio.c | 16 ++++++++++++++++
>> fs/block_dev.c | 2 ++
>> include/linux/fs.h | 17 +++++++++++++++--
>> include/linux/ioprio.h | 2 ++
>> include/uapi/linux/aio_abi.h | 1 +
>> 6 files changed, 52 insertions(+), 8 deletions(-)
>
> This looks fine to me now. I can pick up #1 for 4.18 - and 2+3 as well,
> unless someone else wants to take them.
Great, thanks Jens.
>
^ permalink raw reply
* Re: [RFC PATCH ghak32 V2 01/13] audit: add container id
From: Steve Grubb @ 2018-05-18 13:56 UTC (permalink / raw)
To: Richard Guy Briggs
Cc: simo-H+wXaHxf7aLQT0dZR+AlfA, jlayton-H+wXaHxf7aLQT0dZR+AlfA,
carlos-H+wXaHxf7aLQT0dZR+AlfA, linux-api-u79uwXL29TY76Z2rM5mHXA,
containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, LKML,
eparis-FjpueFixGhCM4zKIHC2jIg, dhowells-H+wXaHxf7aLQT0dZR+AlfA,
Linux-Audit Mailing List, ebiederm-aS9lmoZGLiVWk0Htik3J/w,
luto-DgEjT+Ai2ygdnm+yROfE0A, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
cgroups-u79uwXL29TY76Z2rM5mHXA,
viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn
In-Reply-To: <20180517215600.dyswlkvqdtgjwr5y-bcJWsdo4jJjeVoXN4CMphl7TgLCtbB0G@public.gmane.org>
On Thu, 17 May 2018 17:56:00 -0400
Richard Guy Briggs <rgb-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > During syscall events, the path info is returned in a a record
> > simply called AUDIT_PATH, cwd info is returned in AUDIT_CWD. So,
> > rather than calling the record that gets attached to everything
> > AUDIT_CONTAINER_INFO, how about simply AUDIT_CONTAINER.
>
> Considering the container initiation record is different than the
> record to document the container involved in an otherwise normal
> syscall, we need two names. I don't have a strong opinion what they
> are.
>
> I'd prefer AUDIT_CONTAIN and AUDIT_CONTAINER_INFO so that the two
> are different enough to be visually distinct while leaving
> AUDIT_CONTAINERID for the field type in patch 4 ("audit: add
> containerid filtering")
How about AUDIT_CONTAINER for the auxiliary record? The one that starts
the container, I don't have a strong opinion on. Could be
AUDIT_CONTAINER_INIT, AUDIT_CONTAINER_START, AUDIT_CONTAINERID,
AUDIT_CONTAINER_ID, or something else. The API call that sets the ID
for filtering could be AUDIT_CID or AUDIT_CONTID if that helps decide
what the initial event might be. Normally, it should match the field
being filtered.
Best Regards,
-Steve
^ permalink raw reply
* Re: [PATCH 07/14] powerpc: Add support for restartable sequences
From: Michael Ellerman @ 2018-05-18 12:38 UTC (permalink / raw)
To: Mathieu Desnoyers, Boqun Feng, Will Deacon
Cc: Peter Zijlstra, Paul E. McKenney, Andy Lutomirski, Dave Watson,
linux-kernel, linux-api, Paul Turner, Andrew Morton, Russell King,
Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Andrew Hunter,
Andi Kleen, Chris Lameter, Ben Maurer, rostedt, Josh Triplett,
Linus Torvalds, Catalin Marinas, Michael
In-Reply-To: <277374719.2144.1526570889798.JavaMail.zimbra@efficios.com>
Mathieu Desnoyers <mathieu.desnoyers@efficios.com> writes:
> ----- On May 16, 2018, at 9:19 PM, Boqun Feng boqun.feng@gmail.com wrote:
>> On Wed, May 16, 2018 at 04:13:16PM -0400, Mathieu Desnoyers wrote:
>>> ----- On May 16, 2018, at 12:18 PM, Peter Zijlstra peterz@infradead.org wrote:
>>> > On Mon, Apr 30, 2018 at 06:44:26PM -0400, Mathieu Desnoyers wrote:
>>> >> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>>> >> index c32a181a7cbb..ed21a777e8c6 100644
>>> >> --- a/arch/powerpc/Kconfig
>>> >> +++ b/arch/powerpc/Kconfig
>>> >> @@ -223,6 +223,7 @@ config PPC
>>> >> select HAVE_SYSCALL_TRACEPOINTS
>>> >> select HAVE_VIRT_CPU_ACCOUNTING
>>> >> select HAVE_IRQ_TIME_ACCOUNTING
>>> >> + select HAVE_RSEQ
>>> >> select IRQ_DOMAIN
>>> >> select IRQ_FORCED_THREADING
>>> >> select MODULES_USE_ELF_RELA
>>> >> diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c
>>> >> index 61db86ecd318..d3bb3aaaf5ac 100644
>>> >> --- a/arch/powerpc/kernel/signal.c
>>> >> +++ b/arch/powerpc/kernel/signal.c
>>> >> @@ -133,6 +133,8 @@ static void do_signal(struct task_struct *tsk)
>>> >> /* Re-enable the breakpoints for the signal stack */
>>> >> thread_change_pc(tsk, tsk->thread.regs);
>>> >>
>>> >> + rseq_signal_deliver(tsk->thread.regs);
>>> >> +
>>> >> if (is32) {
>>> >> if (ksig.ka.sa.sa_flags & SA_SIGINFO)
>>> >> ret = handle_rt_signal32(&ksig, oldset, tsk);
>>> >> @@ -164,6 +166,7 @@ void do_notify_resume(struct pt_regs *regs, unsigned long
>>> >> thread_info_flags)
>>> >> if (thread_info_flags & _TIF_NOTIFY_RESUME) {
>>> >> clear_thread_flag(TIF_NOTIFY_RESUME);
>>> >> tracehook_notify_resume(regs);
>>> >> + rseq_handle_notify_resume(regs);
>>> >> }
>>> >>
>>> >> user_enter();
>>> >
>>> > Again no rseq_syscall().
>>>
>>> Same question for PowerPC as for ARM:
>>>
>>> Considering that rseq_syscall is implemented as follows:
>>>
>>> +void rseq_syscall(struct pt_regs *regs)
>>> +{
>>> + unsigned long ip = instruction_pointer(regs);
>>> + struct task_struct *t = current;
>>> + struct rseq_cs rseq_cs;
>>> +
>>> + if (!t->rseq)
>>> + return;
>>> + if (!access_ok(VERIFY_READ, t->rseq, sizeof(*t->rseq)) ||
>>> + rseq_get_rseq_cs(t, &rseq_cs) || in_rseq_cs(ip, &rseq_cs))
>>> + force_sig(SIGSEGV, t);
>>> +}
>>>
>>> and that x86 calls it from syscall_return_slowpath() (which AFAIU is
>>> now used in the fast-path since KPTI), I wonder where we should call
>>
>> So we actually detect this after the syscall takes effect, right? I
>> wonder whether this could be problematic, because "disallowing syscall"
>> in rseq areas may means the syscall won't take effect to some people, I
>> guess?
>>
>>> this on PowerPC ? I was under the impression that PowerPC return to
>>> userspace fast-path was not calling C code unless work flags were set,
>>> but I might be wrong.
>>>
>>
>> I think you're right. So we have to introduce callsite to rseq_syscall()
>> in syscall path, something like:
>>
>> diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
>> index 51695608c68b..a25734a96640 100644
>> --- a/arch/powerpc/kernel/entry_64.S
>> +++ b/arch/powerpc/kernel/entry_64.S
>> @@ -222,6 +222,9 @@ system_call_exit:
>> mtmsrd r11,1
>> #endif /* CONFIG_PPC_BOOK3E */
>>
>> + addi r3,r1,STACK_FRAME_OVERHEAD
>> + bl rseq_syscall
>> +
>> ld r9,TI_FLAGS(r12)
>> li r11,-MAX_ERRNO
>> andi.
>> r0,r9,(_TIF_SYSCALL_DOTRACE|_TIF_SINGLESTEP|_TIF_USER_WORK_MASK|_TIF_PERSYSCALL_MASK)
>>
>> But I think it's important for us to first decide where (before or after
>> the syscall) we do the detection.
>
> As Peter said, we don't really care whether it's on syscall entry or exit, as
> long as the process gets killed when the erroneous use is detected. I think doing
> it on syscall exit is a bit easier because we can clearly access the userspace
> TLS, which AFAIU may be less straightforward on syscall entry.
Coming in to the thread late, sorry if I'm missing the point.
> We may want to add #ifdef CONFIG_DEBUG_RSEQ / #endif around the code you
> proposed above, so it's only compiled in if CONFIG_DEBUG_RSEQ=y.
That sounds good. A function call is not free even if it returns immediately.
> On the ARM leg of the email thread, Will Deacon suggests to test whether current->rseq
> is non-NULL before calling rseq_syscall(). I wonder if this added check is justified
> as the assembly level, considering that this is just a debugging option. We already do
> that check at the very beginning of rseq_syscall().
I guess it depends if this is one of those "debugging options" that's
going to end up turned on in distro kernels?
I think in that code we'd need to check paca->current->rseq, so that
wouldn't be free either.
cheers
^ permalink raw reply
* Re: [PATCH v2 2/4] mm: check for proper migrate type during isolation
From: Vlastimil Babka @ 2018-05-18 10:32 UTC (permalink / raw)
To: Mike Kravetz, linux-mm, linux-kernel, linux-api
Cc: Reinette Chatre, Michal Hocko, Christopher Lameter, Guy Shattah,
Anshuman Khandual, Michal Nazarewicz, David Nellans, Laura Abbott,
Pavel Machek, Dave Hansen, Andrew Morton, Joonsoo Kim
In-Reply-To: <20180503232935.22539-3-mike.kravetz@oracle.com>
On 05/04/2018 01:29 AM, Mike Kravetz wrote:
> The routine start_isolate_page_range and alloc_contig_range have
> comments saying that migratetype must be either MIGRATE_MOVABLE or
> MIGRATE_CMA. However, this is not enforced.
Enforced, no. But if the pageblocks really were as such, it used to
shortcut has_unmovable_pages(). This was wrong and removed in
d7b236e10ced ("mm: drop migrate type checks from has_unmovable_pages")
plus 4da2ce250f98 ("mm: distinguish CMA and MOVABLE isolation in
has_unmovable_pages()").
What is important is
> that that all pageblocks in the range are of type migratetype.
the same
> This is because blocks will be set to migratetype on error.
Strictly speaking this is true only for the CMA case. For other cases,
the best thing actually would be to employ the same heuristics as page
allocation migratetype fallbacks, and count how many pages are free and
how many appear to be movable, see how steal_suitable_fallback() uses
the last parameter of move_freepages_block().
> Add a boolean argument enforce_migratetype to the routine
> start_isolate_page_range. If set, it will check that all pageblocks
> in the range have the passed migratetype. Return -EINVAL is pageblock
if
> is wrong type is found in range.
of
>
> A boolean is used for enforce_migratetype as there are two primary
> users. Contiguous range allocation which wants to enforce migration
> type checking. Memory offline (hotplug) which is not concerned about
> type checking.
This is missing some high-level result. The end change is that CMA is
now enforcing. So we are making it more robust when it's called on
non-CMA pageblocks by mistake? (BTW I still do hope we can remove
MIGRATE_CMA soon after Joonsoo's ZONE_MOVABLE CMA conversion. Combined
with my suggestion above we could hopefully get rid of the migratetype
parameter completely instead of enforcing it?). Is this also a
preparation for introducing find_alloc_contig_pages() which will be
enforcing? (I guess, and will find out shortly, but it should be stated
here)
Thanks,
Vlastimil
> Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
> ---
> include/linux/page-isolation.h | 8 +++-----
> mm/memory_hotplug.c | 2 +-
> mm/page_alloc.c | 17 +++++++++--------
> mm/page_isolation.c | 40 ++++++++++++++++++++++++++++++----------
> 4 files changed, 43 insertions(+), 24 deletions(-)
>
> diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h
> index 4ae347cbc36d..2ab7e5a399ac 100644
> --- a/include/linux/page-isolation.h
> +++ b/include/linux/page-isolation.h
> @@ -38,8 +38,6 @@ int move_freepages_block(struct zone *zone, struct page *page,
>
> /*
> * Changes migrate type in [start_pfn, end_pfn) to be MIGRATE_ISOLATE.
> - * If specified range includes migrate types other than MOVABLE or CMA,
> - * this will fail with -EBUSY.
> *
> * For isolating all pages in the range finally, the caller have to
> * free all pages in the range. test_page_isolated() can be used for
> @@ -47,11 +45,11 @@ int move_freepages_block(struct zone *zone, struct page *page,
> */
> int
> start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
> - unsigned migratetype, bool skip_hwpoisoned_pages);
> + unsigned migratetype, bool skip_hwpoisoned_pages,
> + bool enforce_migratetype);
>
> /*
> - * Changes MIGRATE_ISOLATE to MIGRATE_MOVABLE.
> - * target range is [start_pfn, end_pfn)
> + * Changes MIGRATE_ISOLATE to migratetype for range [start_pfn, end_pfn)
> */
> int
> undo_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index f74826cdceea..ebc1c8c330e2 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1601,7 +1601,7 @@ static int __ref __offline_pages(unsigned long start_pfn,
>
> /* set above range as isolated */
> ret = start_isolate_page_range(start_pfn, end_pfn,
> - MIGRATE_MOVABLE, true);
> + MIGRATE_MOVABLE, true, false);
> if (ret)
> return ret;
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 0fd5e8e2456e..cb1a5e0be6ee 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -7787,9 +7787,10 @@ static int __alloc_contig_migrate_range(struct compact_control *cc,
> * alloc_contig_range() -- tries to allocate given range of pages
> * @start: start PFN to allocate
> * @end: one-past-the-last PFN to allocate
> - * @migratetype: migratetype of the underlaying pageblocks (either
> - * #MIGRATE_MOVABLE or #MIGRATE_CMA). All pageblocks
> - * in range must have the same migratetype and it must
> + * @migratetype: migratetype of the underlaying pageblocks. All
> + * pageblocks in range must have the same migratetype.
> + * migratetype is typically MIGRATE_MOVABLE or
> + * MIGRATE_CMA, but this is not a requirement.
> * be either of the two.
> * @gfp_mask: GFP mask to use during compaction
> *
> @@ -7840,15 +7841,15 @@ int alloc_contig_range(unsigned long start, unsigned long end,
> * allocator removing them from the buddy system. This way
> * page allocator will never consider using them.
> *
> - * This lets us mark the pageblocks back as
> - * MIGRATE_CMA/MIGRATE_MOVABLE so that free pages in the
> - * aligned range but not in the unaligned, original range are
> - * put back to page allocator so that buddy can use them.
> + * This lets us mark the pageblocks back as their original
> + * migrate type so that free pages in the aligned range but
> + * not in the unaligned, original range are put back to page
> + * allocator so that buddy can use them.
> */
>
> ret = start_isolate_page_range(pfn_max_align_down(start),
> pfn_max_align_up(end), migratetype,
> - false);
> + false, true);
> if (ret)
> return ret;
>
> diff --git a/mm/page_isolation.c b/mm/page_isolation.c
> index 43e085608846..472191cc1909 100644
> --- a/mm/page_isolation.c
> +++ b/mm/page_isolation.c
> @@ -16,7 +16,8 @@
> #include <trace/events/page_isolation.h>
>
> static int set_migratetype_isolate(struct page *page, int migratetype,
> - bool skip_hwpoisoned_pages)
> + bool skip_hwpoisoned_pages,
> + bool enforce_migratetype)
> {
> struct zone *zone;
> unsigned long flags, pfn;
> @@ -36,6 +37,17 @@ static int set_migratetype_isolate(struct page *page, int migratetype,
> if (is_migrate_isolate_page(page))
> goto out;
>
> + /*
> + * If requested, check migration type of pageblock and make sure
> + * it matches migratetype
> + */
> + if (enforce_migratetype) {
> + if (get_pageblock_migratetype(page) != migratetype) {
> + ret = -EINVAL;
> + goto out;
> + }
> + }
> +
> pfn = page_to_pfn(page);
> arg.start_pfn = pfn;
> arg.nr_pages = pageblock_nr_pages;
> @@ -167,14 +179,16 @@ __first_valid_page(unsigned long pfn, unsigned long nr_pages)
> * to be MIGRATE_ISOLATE.
> * @start_pfn: The lower PFN of the range to be isolated.
> * @end_pfn: The upper PFN of the range to be isolated.
> - * @migratetype: migrate type to set in error recovery.
> + * @migratetype: migrate type of all blocks in range.
> *
> * Making page-allocation-type to be MIGRATE_ISOLATE means free pages in
> * the range will never be allocated. Any free pages and pages freed in the
> * future will not be allocated again.
> *
> * start_pfn/end_pfn must be aligned to pageblock_order.
> - * Return 0 on success and -EBUSY if any part of range cannot be isolated.
> + * Return 0 on success or error returned by set_migratetype_isolate. Typical
> + * errors are -EBUSY if any part of range cannot be isolated or -EINVAL if
> + * any page block is not of migratetype.
> *
> * There is no high level synchronization mechanism that prevents two threads
> * from trying to isolate overlapping ranges. If this happens, one thread
> @@ -185,11 +199,13 @@ __first_valid_page(unsigned long pfn, unsigned long nr_pages)
> * prevents two threads from simultaneously working on overlapping ranges.
> */
> int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
> - unsigned migratetype, bool skip_hwpoisoned_pages)
> + unsigned migratetype, bool skip_hwpoisoned_pages,
> + bool enforce_migratetype)
> {
> unsigned long pfn;
> unsigned long undo_pfn;
> struct page *page;
> + int ret = 0;
>
> BUG_ON(!IS_ALIGNED(start_pfn, pageblock_nr_pages));
> BUG_ON(!IS_ALIGNED(end_pfn, pageblock_nr_pages));
> @@ -198,13 +214,17 @@ int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
> pfn < end_pfn;
> pfn += pageblock_nr_pages) {
> page = __first_valid_page(pfn, pageblock_nr_pages);
> - if (page &&
> - set_migratetype_isolate(page, migratetype, skip_hwpoisoned_pages)) {
> - undo_pfn = pfn;
> - goto undo;
> + if (page) {
> + ret = set_migratetype_isolate(page, migratetype,
> + skip_hwpoisoned_pages,
> + enforce_migratetype);
> + if (ret) {
> + undo_pfn = pfn;
> + goto undo;
> + }
> }
> }
> - return 0;
> + return ret;
> undo:
> for (pfn = start_pfn;
> pfn < undo_pfn;
> @@ -215,7 +235,7 @@ int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
> unset_migratetype_isolate(page, migratetype);
> }
>
> - return -EBUSY;
> + return ret;
> }
>
> /*
>
^ permalink raw reply
* Re: [PATCH v2 1/4] mm: change type of free_contig_range(nr_pages) to unsigned long
From: Vlastimil Babka @ 2018-05-18 9:12 UTC (permalink / raw)
To: Mike Kravetz, linux-mm, linux-kernel, linux-api
Cc: Reinette Chatre, Michal Hocko, Christopher Lameter, Guy Shattah,
Anshuman Khandual, Michal Nazarewicz, David Nellans, Laura Abbott,
Pavel Machek, Dave Hansen, Andrew Morton, Joonsoo Kim
In-Reply-To: <20180503232935.22539-2-mike.kravetz@oracle.com>
On 05/04/2018 01:29 AM, Mike Kravetz wrote:
> free_contig_range() is currently defined as:
> void free_contig_range(unsigned long pfn, unsigned nr_pages);
> change to,
> void free_contig_range(unsigned long pfn, unsigned long nr_pages);
>
> Some callers are passing a truncated unsigned long today. It is
> highly unlikely that these values will overflow an unsigned int.
> However, this should be changed to an unsigned long to be consistent
> with other page counts.
>
> Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
> ---
> include/linux/gfp.h | 2 +-
> mm/cma.c | 2 +-
> mm/hugetlb.c | 2 +-
> mm/page_alloc.c | 6 +++---
> 4 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index 1a4582b44d32..86a0d06463ab 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -572,7 +572,7 @@ static inline bool pm_suspended_storage(void)
> /* The below functions must be run on a range from a single zone. */
> extern int alloc_contig_range(unsigned long start, unsigned long end,
> unsigned migratetype, gfp_t gfp_mask);
> -extern void free_contig_range(unsigned long pfn, unsigned nr_pages);
> +extern void free_contig_range(unsigned long pfn, unsigned long nr_pages);
> #endif
>
> #ifdef CONFIG_CMA
> diff --git a/mm/cma.c b/mm/cma.c
> index aa40e6c7b042..f473fc2b7cbd 100644
> --- a/mm/cma.c
> +++ b/mm/cma.c
> @@ -563,7 +563,7 @@ bool cma_release(struct cma *cma, const struct page *pages, unsigned int count)
>
> VM_BUG_ON(pfn + count > cma->base_pfn + cma->count);
>
> - free_contig_range(pfn, count);
> + free_contig_range(pfn, (unsigned long)count);
I guess this cast from uint to ulong doesn't need to be explicit? But
instead, cma_release() signature could be also changed to ulong, because
some of its callers do pass those?
> cma_clear_bitmap(cma, pfn, count);
> trace_cma_release(pfn, pages, count);
>
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 218679138255..c81072ce7510 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -1055,7 +1055,7 @@ static void destroy_compound_gigantic_page(struct page *page,
>
> static void free_gigantic_page(struct page *page, unsigned int order)
> {
> - free_contig_range(page_to_pfn(page), 1 << order);
> + free_contig_range(page_to_pfn(page), 1UL << order);
> }
>
> static int __alloc_gigantic_page(unsigned long start_pfn,
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 905db9d7962f..0fd5e8e2456e 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -7937,9 +7937,9 @@ int alloc_contig_range(unsigned long start, unsigned long end,
> return ret;
> }
>
> -void free_contig_range(unsigned long pfn, unsigned nr_pages)
> +void free_contig_range(unsigned long pfn, unsigned long nr_pages)
> {
> - unsigned int count = 0;
> + unsigned long count = 0;
>
> for (; nr_pages--; pfn++) {
> struct page *page = pfn_to_page(pfn);
> @@ -7947,7 +7947,7 @@ void free_contig_range(unsigned long pfn, unsigned nr_pages)
> count += page_count(page) != 1;
> __free_page(page);
> }
> - WARN(count != 0, "%d pages are still in use!\n", count);
> + WARN(count != 0, "%ld pages are still in use!\n", count);
Maybe change to %lu while at it?
BTW, this warning can theoretically produce false positives, because
page users have to deal with page_count() being incremented by e.g.
parallel pfn scanners using get_page_unless_zero(). We also don't detect
refcount leaks in general. Should we remove it or change it to VM_WARN
if it's still useful for debugging?
> }
> #endif
>
>
^ permalink raw reply
* Re: [PATCH 7/7] aio: implement io_pgetevents
From: Christoph Hellwig @ 2018-05-18 8:57 UTC (permalink / raw)
To: James Hogan
Cc: Christoph Hellwig, viro, Avi Kivity, linux-aio, linux-fsdevel,
linux-api, linux-kernel
In-Reply-To: <20180518082837.GA3264@jamesdev>
On Fri, May 18, 2018 at 09:28:38AM +0100, James Hogan wrote:
> Given this:
>
> On Wed, May 02, 2018 at 11:14:48PM +0200, Christoph Hellwig wrote:
> > +struct __aio_sigset {
> > + sigset_t __user *sigmask;
> > + size_t sigsetsize;
> > +};
>
> and:
>
> > +asmlinkage long sys_io_pgetevents(aio_context_t ctx_id,
> > + long min_nr,
> > + long nr,
> > + struct io_event __user *events,
> > + struct timespec __user *timeout,
> > + const struct __aio_sigset *sig);
>
> The following paragraph in the commit message would appear to be
> misleading since __aio_sigset contains a size:
>
> > Note that unlike many other signal related calls we do not pass a sigmask
> > size, as that would get us to 7 arguments, which aren't easily supported
> > by the syscall infrastructure. It seems a lot less painful to just add a
> > new syscall variant in the unlikely case we're going to increase the
> > sigset size.
>
> Is it possible to correct it before this gets merged?
True, the calling convention change based on feedback. Al, is the
branch stable or you can edit this out?
^ permalink raw reply
* Re: [PATCH 7/7] aio: implement io_pgetevents
From: James Hogan @ 2018-05-18 8:28 UTC (permalink / raw)
To: Christoph Hellwig
Cc: viro, Avi Kivity, linux-aio, linux-fsdevel, linux-api,
linux-kernel
In-Reply-To: <20180502211448.18276-8-hch@lst.de>
[-- Attachment #1: Type: text/plain, Size: 882 bytes --]
Given this:
On Wed, May 02, 2018 at 11:14:48PM +0200, Christoph Hellwig wrote:
> +struct __aio_sigset {
> + sigset_t __user *sigmask;
> + size_t sigsetsize;
> +};
and:
> +asmlinkage long sys_io_pgetevents(aio_context_t ctx_id,
> + long min_nr,
> + long nr,
> + struct io_event __user *events,
> + struct timespec __user *timeout,
> + const struct __aio_sigset *sig);
The following paragraph in the commit message would appear to be
misleading since __aio_sigset contains a size:
> Note that unlike many other signal related calls we do not pass a sigmask
> size, as that would get us to 7 arguments, which aren't easily supported
> by the syscall infrastructure. It seems a lot less painful to just add a
> new syscall variant in the unlikely case we're going to increase the
> sigset size.
Is it possible to correct it before this gets merged?
Thanks
James
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v4 0/3] AIO add per-command iopriority
From: Jens Axboe @ 2018-05-18 2:41 UTC (permalink / raw)
To: adam.manzanares, viro, linux-fsdevel, bcrl
Cc: mingo, peterz, pombredanne, gregkh, bigeasy, rgoldwyn,
linux-block, linux-kernel, linux-aio, linux-api
In-Reply-To: <20180517203803.2664-1-adam.manzanares@wdc.com>
On 5/17/18 2:38 PM, adam.manzanares@wdc.com wrote:
> From: Adam Manzanares <adam.manzanares@wdc.com>
>
> This is the per-I/O equivalent of the ioprio_set system call.
> See the following link for performance implications on a SATA HDD:
> https://lkml.org/lkml/2016/12/6/495
>
> First patch factors ioprio_check_cap function out of ioprio_set system call to
> also be used by the aio ioprio interface.
>
> Second patch converts kiocb ki_hint field to a u16 to avoid kiocb bloat.
>
> Third patch passes ioprio hint from aio iocb to kiocb and enables block_dev
> usage of the per I/O ioprio feature.
>
> v2: merge patches
> use IOCB_FLAG_IOPRIO
> validate intended use with IOCB_IOPRIO
> add linux-api and linux-block to cc
>
> v3: add ioprio_check_cap function
> convert kiocb ki_hint to u16
> use ioprio_check_cap when adding ioprio to kiocb in aio.c
>
> v4: handle IOCB_IOPRIO in aio_prep_rw
> note patch 3 depends on patch 1 in commit msg
>
> Adam Manzanares (3):
> block: add ioprio_check_cap function
> fs: Convert kiocb rw_hint from enum to u16
> fs: Add aio iopriority support for block_dev
>
> block/ioprio.c | 22 ++++++++++++++++------
> fs/aio.c | 16 ++++++++++++++++
> fs/block_dev.c | 2 ++
> include/linux/fs.h | 17 +++++++++++++++--
> include/linux/ioprio.h | 2 ++
> include/uapi/linux/aio_abi.h | 1 +
> 6 files changed, 52 insertions(+), 8 deletions(-)
This looks fine to me now. I can pick up #1 for 4.18 - and 2+3 as well,
unless someone else wants to take them.
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH 07/14] powerpc: Add support for restartable sequences
From: Boqun Feng @ 2018-05-17 23:50 UTC (permalink / raw)
To: Mathieu Desnoyers, Will Deacon
Cc: Peter Zijlstra, Paul E. McKenney, Andy Lutomirski, Dave Watson,
linux-kernel, linux-api, Paul Turner, Andrew Morton, Russell King,
Thomas Gleixner, Ingo Molnar, H. Peter Anvin, Andrew Hunter,
Andi Kleen, Chris Lameter, Ben Maurer, rostedt, Josh Triplett,
Linus Torvalds, Catalin Marinas, Michael
In-Reply-To: <277374719.2144.1526570889798.JavaMail.zimbra@efficios.com>
On Thu, May 17, 2018, at 11:28 PM, Mathieu Desnoyers wrote:
> ----- On May 16, 2018, at 9:19 PM, Boqun Feng boqun.feng@gmail.com wrote:
>
> > On Wed, May 16, 2018 at 04:13:16PM -0400, Mathieu Desnoyers wrote:
> >> ----- On May 16, 2018, at 12:18 PM, Peter Zijlstra peterz@infradead.org wrote:
> >>
> >> > On Mon, Apr 30, 2018 at 06:44:26PM -0400, Mathieu Desnoyers wrote:
> >> >> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> >> >> index c32a181a7cbb..ed21a777e8c6 100644
> >> >> --- a/arch/powerpc/Kconfig
> >> >> +++ b/arch/powerpc/Kconfig
> >> >> @@ -223,6 +223,7 @@ config PPC
> >> >> select HAVE_SYSCALL_TRACEPOINTS
> >> >> select HAVE_VIRT_CPU_ACCOUNTING
> >> >> select HAVE_IRQ_TIME_ACCOUNTING
> >> >> + select HAVE_RSEQ
> >> >> select IRQ_DOMAIN
> >> >> select IRQ_FORCED_THREADING
> >> >> select MODULES_USE_ELF_RELA
> >> >> diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c
> >> >> index 61db86ecd318..d3bb3aaaf5ac 100644
> >> >> --- a/arch/powerpc/kernel/signal.c
> >> >> +++ b/arch/powerpc/kernel/signal.c
> >> >> @@ -133,6 +133,8 @@ static void do_signal(struct task_struct *tsk)
> >> >> /* Re-enable the breakpoints for the signal stack */
> >> >> thread_change_pc(tsk, tsk->thread.regs);
> >> >>
> >> >> + rseq_signal_deliver(tsk->thread.regs);
> >> >> +
> >> >> if (is32) {
> >> >> if (ksig.ka.sa.sa_flags & SA_SIGINFO)
> >> >> ret = handle_rt_signal32(&ksig, oldset, tsk);
> >> >> @@ -164,6 +166,7 @@ void do_notify_resume(struct pt_regs *regs, unsigned long
> >> >> thread_info_flags)
> >> >> if (thread_info_flags & _TIF_NOTIFY_RESUME) {
> >> >> clear_thread_flag(TIF_NOTIFY_RESUME);
> >> >> tracehook_notify_resume(regs);
> >> >> + rseq_handle_notify_resume(regs);
> >> >> }
> >> >>
> >> >> user_enter();
> >> >
> >> > Again no rseq_syscall().
> >>
> >> Same question for PowerPC as for ARM:
> >>
> >> Considering that rseq_syscall is implemented as follows:
> >>
> >> +void rseq_syscall(struct pt_regs *regs)
> >> +{
> >> + unsigned long ip = instruction_pointer(regs);
> >> + struct task_struct *t = current;
> >> + struct rseq_cs rseq_cs;
> >> +
> >> + if (!t->rseq)
> >> + return;
> >> + if (!access_ok(VERIFY_READ, t->rseq, sizeof(*t->rseq)) ||
> >> + rseq_get_rseq_cs(t, &rseq_cs) || in_rseq_cs(ip, &rseq_cs))
> >> + force_sig(SIGSEGV, t);
> >> +}
> >>
> >> and that x86 calls it from syscall_return_slowpath() (which AFAIU is
> >> now used in the fast-path since KPTI), I wonder where we should call
> >
> > So we actually detect this after the syscall takes effect, right? I
> > wonder whether this could be problematic, because "disallowing syscall"
> > in rseq areas may means the syscall won't take effect to some people, I
> > guess?
> >
> >> this on PowerPC ? I was under the impression that PowerPC return to
> >> userspace fast-path was not calling C code unless work flags were set,
> >> but I might be wrong.
> >>
> >
> > I think you're right. So we have to introduce callsite to rseq_syscall()
> > in syscall path, something like:
> >
> > diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
> > index 51695608c68b..a25734a96640 100644
> > --- a/arch/powerpc/kernel/entry_64.S
> > +++ b/arch/powerpc/kernel/entry_64.S
> > @@ -222,6 +222,9 @@ system_call_exit:
> > mtmsrd r11,1
> > #endif /* CONFIG_PPC_BOOK3E */
> >
> > + addi r3,r1,STACK_FRAME_OVERHEAD
> > + bl rseq_syscall
> > +
> > ld r9,TI_FLAGS(r12)
> > li r11,-MAX_ERRNO
> > andi.
> > r0,r9,(_TIF_SYSCALL_DOTRACE|_TIF_SINGLESTEP|_TIF_USER_WORK_MASK|_TIF_PERSYSCALL_MASK)
> >
> > But I think it's important for us to first decide where (before or after
> > the syscall) we do the detection.
>
> As Peter said, we don't really care whether it's on syscall entry or
> exit, as
> long as the process gets killed when the erroneous use is detected. I
> think doing
> it on syscall exit is a bit easier because we can clearly access the
> userspace
> TLS, which AFAIU may be less straightforward on syscall entry.
>
Fair enough.
> We may want to add #ifdef CONFIG_DEBUG_RSEQ / #endif around the code you
> proposed above, so it's only compiled in if CONFIG_DEBUG_RSEQ=y.
>
OK.
> On the ARM leg of the email thread, Will Deacon suggests to test whether
> current->rseq
> is non-NULL before calling rseq_syscall(). I wonder if this added check
> is justified
> as the assembly level, considering that this is just a debugging option.
> We already do
> that check at the very beginning of rseq_syscall().
>
Yes, I think it's better to do the check in rseq_syscall(), leaving the asm
code a bit cleaner.
Regards,
Boqun
> Thoughts ?
>
> Thanks,
>
> Mathieu
>
> >
> > Regards,
> > Boqun
> >
> >> Thoughts ?
> >>
> >> Thanks!
> >>
> >> Mathieu
> >>
> >> --
> >> Mathieu Desnoyers
> >> EfficiOS Inc.
> > > http://www.efficios.com
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com
^ permalink raw reply
* Re: [RFC PATCH ghak32 V2 01/13] audit: add container id
From: Richard Guy Briggs @ 2018-05-17 21:56 UTC (permalink / raw)
To: Steve Grubb
Cc: cgroups, containers, linux-api, Linux-Audit Mailing List,
linux-fsdevel, LKML, netdev, ebiederm, luto, jlayton, carlos,
dhowells, viro, simo, eparis, serge
In-Reply-To: <20180517170053.7d4afa87@ivy-bridge>
On 2018-05-17 17:00, Steve Grubb wrote:
> On Fri, 16 Mar 2018 05:00:28 -0400
> Richard Guy Briggs <rgb@redhat.com> wrote:
>
> > Implement the proc fs write to set the audit container ID of a
> > process, emitting an AUDIT_CONTAINER record to document the event.
> >
> > This is a write from the container orchestrator task to a proc entry
> > of the form /proc/PID/containerid where PID is the process ID of the
> > newly created task that is to become the first task in a container,
> > or an additional task added to a container.
> >
> > The write expects up to a u64 value (unset: 18446744073709551615).
> >
> > This will produce a record such as this:
> > type=CONTAINER msg=audit(1519903238.968:261): op=set pid=596 uid=0
> > subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 auid=0
> > tty=pts0 ses=1 opid=596 old-contid=18446744073709551615 contid=123455
> > res=0
>
> The was one thing I was wondering about. Currently when we set the
> loginuid, the record is AUDIT_LOGINUID. The corollary is that when we
> set the container id, the event should be AUDIT_CONTAINERID or
> AUDIT_CONTAINER_ID.
The record type is actually AUDIT_LOGIN. The field type is
AUDIT_LOGINUID. Given that correction, I think we're fine and could
potentially violently agree. The existing naming is consistent.
> During syscall events, the path info is returned in a a record simply
> called AUDIT_PATH, cwd info is returned in AUDIT_CWD. So, rather than
> calling the record that gets attached to everything
> AUDIT_CONTAINER_INFO, how about simply AUDIT_CONTAINER.
Considering the container initiation record is different than the record
to document the container involved in an otherwise normal syscall, we
need two names. I don't have a strong opinion what they are.
I'd prefer AUDIT_CONTAINER and AUDIT_CONTAINER_INFO so that the two are
different enough to be visually distinct while leaving
AUDIT_CONTAINERID for the field type in patch 4 ("audit: add containerid
filtering")
> > The "op" field indicates an initial set. The "pid" to "ses" fields
> > are the orchestrator while the "opid" field is the object's PID, the
> > process being "contained". Old and new container ID values are given
> > in the "contid" fields, while res indicates its success.
> >
> > It is not permitted to self-set, unset or re-set the container ID. A
> > child inherits its parent's container ID, but then can be set only
> > once after.
> >
> > See: https://github.com/linux-audit/audit-kernel/issues/32
> >
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > ---
> > fs/proc/base.c | 37 ++++++++++++++++++++
> > include/linux/audit.h | 16 +++++++++
> > include/linux/init_task.h | 4 ++-
> > include/linux/sched.h | 1 +
> > include/uapi/linux/audit.h | 2 ++
> > kernel/auditsc.c | 84
> > ++++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 143
> > insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/proc/base.c b/fs/proc/base.c
> > index 60316b5..6ce4fbe 100644
> > --- a/fs/proc/base.c
> > +++ b/fs/proc/base.c
> > @@ -1299,6 +1299,41 @@ static ssize_t proc_sessionid_read(struct file
> > * file, char __user * buf, .read = proc_sessionid_read,
> > .llseek = generic_file_llseek,
> > };
> > +
> > +static ssize_t proc_containerid_write(struct file *file, const char
> > __user *buf,
> > + size_t count, loff_t *ppos)
> > +{
> > + struct inode *inode = file_inode(file);
> > + u64 containerid;
> > + int rv;
> > + struct task_struct *task = get_proc_task(inode);
> > +
> > + if (!task)
> > + return -ESRCH;
> > + if (*ppos != 0) {
> > + /* No partial writes. */
> > + put_task_struct(task);
> > + return -EINVAL;
> > + }
> > +
> > + rv = kstrtou64_from_user(buf, count, 10, &containerid);
> > + if (rv < 0) {
> > + put_task_struct(task);
> > + return rv;
> > + }
> > +
> > + rv = audit_set_containerid(task, containerid);
> > + put_task_struct(task);
> > + if (rv < 0)
> > + return rv;
> > + return count;
> > +}
> > +
> > +static const struct file_operations proc_containerid_operations = {
> > + .write = proc_containerid_write,
> > + .llseek = generic_file_llseek,
> > +};
> > +
> > #endif
> >
> > #ifdef CONFIG_FAULT_INJECTION
> > @@ -2961,6 +2996,7 @@ static int proc_pid_patch_state(struct seq_file
> > *m, struct pid_namespace *ns, #ifdef CONFIG_AUDITSYSCALL
> > REG("loginuid", S_IWUSR|S_IRUGO, proc_loginuid_operations),
> > REG("sessionid", S_IRUGO, proc_sessionid_operations),
> > + REG("containerid", S_IWUSR, proc_containerid_operations),
> > #endif
> > #ifdef CONFIG_FAULT_INJECTION
> > REG("make-it-fail", S_IRUGO|S_IWUSR,
> > proc_fault_inject_operations), @@ -3355,6 +3391,7 @@ static int
> > proc_tid_comm_permission(struct inode *inode, int mask) #ifdef
> > CONFIG_AUDITSYSCALL REG("loginuid", S_IWUSR|S_IRUGO,
> > proc_loginuid_operations), REG("sessionid", S_IRUGO,
> > proc_sessionid_operations),
> > + REG("containerid", S_IWUSR, proc_containerid_operations),
> > #endif
> > #ifdef CONFIG_FAULT_INJECTION
> > REG("make-it-fail", S_IRUGO|S_IWUSR,
> > proc_fault_inject_operations), diff --git a/include/linux/audit.h
> > b/include/linux/audit.h index af410d9..fe4ba3f 100644
> > --- a/include/linux/audit.h
> > +++ b/include/linux/audit.h
> > @@ -29,6 +29,7 @@
> >
> > #define AUDIT_INO_UNSET ((unsigned long)-1)
> > #define AUDIT_DEV_UNSET ((dev_t)-1)
> > +#define INVALID_CID AUDIT_CID_UNSET
> >
> > struct audit_sig_info {
> > uid_t uid;
> > @@ -321,6 +322,7 @@ static inline void audit_ptrace(struct
> > task_struct *t) extern int auditsc_get_stamp(struct audit_context
> > *ctx, struct timespec64 *t, unsigned int *serial);
> > extern int audit_set_loginuid(kuid_t loginuid);
> > +extern int audit_set_containerid(struct task_struct *tsk, u64
> > containerid);
> > static inline kuid_t audit_get_loginuid(struct task_struct *tsk)
> > {
> > @@ -332,6 +334,11 @@ static inline unsigned int
> > audit_get_sessionid(struct task_struct *tsk) return tsk->sessionid;
> > }
> >
> > +static inline u64 audit_get_containerid(struct task_struct *tsk)
> > +{
> > + return tsk->containerid;
> > +}
> > +
> > extern void __audit_ipc_obj(struct kern_ipc_perm *ipcp);
> > extern void __audit_ipc_set_perm(unsigned long qbytes, uid_t uid,
> > gid_t gid, umode_t mode); extern void __audit_bprm(struct
> > linux_binprm *bprm); @@ -517,6 +524,10 @@ static inline unsigned int
> > audit_get_sessionid(struct task_struct *tsk) {
> > return -1;
> > }
> > +static inline kuid_t audit_get_containerid(struct task_struct *tsk)
> > +{
> > + return INVALID_CID;
> > +}
> > static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp)
> > { }
> > static inline void audit_ipc_set_perm(unsigned long qbytes, uid_t
> > uid, @@ -581,6 +592,11 @@ static inline bool
> > audit_loginuid_set(struct task_struct *tsk) return
> > uid_valid(audit_get_loginuid(tsk)); }
> >
> > +static inline bool audit_containerid_set(struct task_struct *tsk)
> > +{
> > + return audit_get_containerid(tsk) != INVALID_CID;
> > +}
> > +
> > static inline void audit_log_string(struct audit_buffer *ab, const
> > char *buf) {
> > audit_log_n_string(ab, buf, strlen(buf));
> > diff --git a/include/linux/init_task.h b/include/linux/init_task.h
> > index 6a53262..046bd0a 100644
> > --- a/include/linux/init_task.h
> > +++ b/include/linux/init_task.h
> > @@ -18,6 +18,7 @@
> > #include <linux/sched/rt.h>
> > #include <linux/livepatch.h>
> > #include <linux/mm_types.h>
> > +#include <linux/audit.h>
> >
> > #include <asm/thread_info.h>
> >
> > @@ -120,7 +121,8 @@
> > #ifdef CONFIG_AUDITSYSCALL
> > #define INIT_IDS \
> > .loginuid = INVALID_UID, \
> > - .sessionid = (unsigned int)-1,
> > + .sessionid = (unsigned int)-1, \
> > + .containerid = INVALID_CID,
> > #else
> > #define INIT_IDS
> > #endif
> > diff --git a/include/linux/sched.h b/include/linux/sched.h
> > index d258826..1b82191 100644
> > --- a/include/linux/sched.h
> > +++ b/include/linux/sched.h
> > @@ -796,6 +796,7 @@ struct task_struct {
> > #ifdef CONFIG_AUDITSYSCALL
> > kuid_t loginuid;
> > unsigned int sessionid;
> > + u64 containerid;
> > #endif
> > struct seccomp seccomp;
> >
> > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> > index 4e61a9e..921a71f 100644
> > --- a/include/uapi/linux/audit.h
> > +++ b/include/uapi/linux/audit.h
> > @@ -71,6 +71,7 @@
> > #define AUDIT_TTY_SET 1017 /* Set TTY auditing
> > status */ #define AUDIT_SET_FEATURE 1018 /* Turn an
> > audit feature on or off */ #define AUDIT_GET_FEATURE
> > 1019 /* Get which features are enabled */ +#define
> > AUDIT_CONTAINER 1020 /* Define the container id
> > and information */ #define AUDIT_FIRST_USER_MSG 1100 /*
> > Userspace messages mostly uninteresting to kernel */ #define
> > AUDIT_USER_AVC 1107 /* We filter this
> > differently */ @@ -465,6 +466,7 @@ struct audit_tty_status { };
> >
> > #define AUDIT_UID_UNSET (unsigned int)-1
> > +#define AUDIT_CID_UNSET ((u64)-1)
> >
> > /* audit_rule_data supports filter rules with both integer and string
> > * fields. It corresponds with AUDIT_ADD_RULE, AUDIT_DEL_RULE and
> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > index 4e0a4ac..29c8482 100644
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -2073,6 +2073,90 @@ int audit_set_loginuid(kuid_t loginuid)
> > return rc;
> > }
> >
> > +static int audit_set_containerid_perm(struct task_struct *task, u64
> > containerid) +{
> > + struct task_struct *parent;
> > + u64 pcontainerid, ccontainerid;
> > +
> > + /* Don't allow to set our own containerid */
> > + if (current == task)
> > + return -EPERM;
> > + /* Don't allow the containerid to be unset */
> > + if (!cid_valid(containerid))
> > + return -EINVAL;
> > + /* if we don't have caps, reject */
> > + if (!capable(CAP_AUDIT_CONTROL))
> > + return -EPERM;
> > + /* if containerid is unset, allow */
> > + if (!audit_containerid_set(task))
> > + return 0;
> > + /* it is already set, and not inherited from the parent,
> > reject */
> > + ccontainerid = audit_get_containerid(task);
> > + rcu_read_lock();
> > + parent = rcu_dereference(task->real_parent);
> > + rcu_read_unlock();
> > + task_lock(parent);
> > + pcontainerid = audit_get_containerid(parent);
> > + task_unlock(parent);
> > + if (ccontainerid != pcontainerid)
> > + return -EPERM;
> > + return 0;
> > +}
> > +
> > +static void audit_log_set_containerid(struct task_struct *task, u64
> > oldcontainerid,
> > + u64 containerid, int rc)
> > +{
> > + struct audit_buffer *ab;
> > + uid_t uid;
> > + struct tty_struct *tty;
> > +
> > + if (!audit_enabled)
> > + return;
> > +
> > + ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONTAINER);
> > + if (!ab)
> > + return;
> > +
> > + uid = from_kuid(&init_user_ns, task_uid(current));
> > + tty = audit_get_tty(current);
> > +
> > + audit_log_format(ab, "op=set pid=%d uid=%u",
> > task_tgid_nr(current), uid);
> > + audit_log_task_context(ab);
> > + audit_log_format(ab, " auid=%u tty=%s ses=%u opid=%d
> > old-contid=%llu contid=%llu res=%d",
>
> The preferred ordering would be: op, opid, old-contid, contid, pid, uid,
> tty, ses, subj, comm, exe, res. This groups the searchable fields
> together using the most common ordering so that parsing is simple.
There has been a suggestion to make this a syscall-connected record, and
if that is the case, we'd simply drop all the fields that would be
duplicated by the syscall record. Otherwise, I'll use your suggested
order.
As you may recall this suggestion was also made for the AUDIT_LOGIN
record.
> Thanks,
> -Steve
>
> > + from_kuid(&init_user_ns,
> > audit_get_loginuid(current)),
> > + tty ? tty_name(tty) : "(none)",
> > audit_get_sessionid(current),
> > + task_tgid_nr(task), oldcontainerid,
> > containerid, !rc); +
> > + audit_put_tty(tty);
> > + audit_log_end(ab);
> > +}
> > +
> > +/**
> > + * audit_set_containerid - set current task's audit_context
> > containerid
> > + * @containerid: containerid value
> > + *
> > + * Returns 0 on success, -EPERM on permission failure.
> > + *
> > + * Called (set) from fs/proc/base.c::proc_containerid_write().
> > + */
> > +int audit_set_containerid(struct task_struct *task, u64 containerid)
> > +{
> > + u64 oldcontainerid;
> > + int rc;
> > +
> > + oldcontainerid = audit_get_containerid(task);
> > +
> > + rc = audit_set_containerid_perm(task, containerid);
> > + if (!rc) {
> > + task_lock(task);
> > + task->containerid = containerid;
> > + task_unlock(task);
> > + }
> > +
> > + audit_log_set_containerid(task, oldcontainerid, containerid,
> > rc);
> > + return rc;
> > +}
> > +
> > /**
> > * __audit_mq_open - record audit data for a POSIX MQ open
> > * @oflag: open flag
>
- RGB
--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
^ permalink raw reply
* Re: [RFC PATCH ghak32 V2 03/13] audit: log container info of syscalls
From: Richard Guy Briggs @ 2018-05-17 21:41 UTC (permalink / raw)
To: Steve Grubb
Cc: cgroups, containers, linux-api, Linux-Audit Mailing List,
linux-fsdevel, LKML, netdev, ebiederm, luto, jlayton, carlos,
dhowells, viro, simo, eparis, serge
In-Reply-To: <20180517170907.3d9f7c1a@ivy-bridge>
On 2018-05-17 17:09, Steve Grubb wrote:
> On Fri, 16 Mar 2018 05:00:30 -0400
> Richard Guy Briggs <rgb@redhat.com> wrote:
>
> > Create a new audit record AUDIT_CONTAINER_INFO to document the
> > container ID of a process if it is present.
>
> As mentioned in a previous email, I think AUDIT_CONTAINER is more
> suitable for the container record. One more comment below...
>
> > Called from audit_log_exit(), syscalls are covered.
> >
> > A sample raw event:
> > type=SYSCALL msg=audit(1519924845.499:257): arch=c000003e syscall=257
> > success=yes exit=3 a0=ffffff9c a1=56374e1cef30 a2=241 a3=1b6 items=2
> > ppid=606 pid=635 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0
> > sgid=0 fsgid=0 tty=pts0 ses=3 comm="bash" exe="/usr/bin/bash"
> > subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
> > key="tmpcontainerid" type=CWD msg=audit(1519924845.499:257):
> > cwd="/root" type=PATH msg=audit(1519924845.499:257): item=0
> > name="/tmp/" inode=13863 dev=00:27 mode=041777 ouid=0 ogid=0
> > rdev=00:00 obj=system_u:object_r:tmp_t:s0 nametype= PARENT
> > cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
> > type=PATH msg=audit(1519924845.499:257): item=1
> > name="/tmp/tmpcontainerid" inode=17729 dev=00:27 mode=0100644 ouid=0
> > ogid=0 rdev=00:00 obj=unconfined_u:object_r:user_tmp_t:s0
> > nametype=CREATE cap_fp=0000000000000000 cap_fi=0000000000000000
> > cap_fe=0 cap_fver=0 type=PROCTITLE msg=audit(1519924845.499:257):
> > proctitle=62617368002D6300736C65657020313B206563686F2074657374203E202F746D702F746D70636F6E7461696E65726964
> > type=CONTAINER_INFO msg=audit(1519924845.499:257): op=task
> > contid=123458
> >
> > See: https://github.com/linux-audit/audit-kernel/issues/32
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > ---
> > include/linux/audit.h | 5 +++++
> > include/uapi/linux/audit.h | 1 +
> > kernel/audit.c | 20 ++++++++++++++++++++
> > kernel/auditsc.c | 2 ++
> > 4 files changed, 28 insertions(+)
> >
> > diff --git a/include/linux/audit.h b/include/linux/audit.h
> > index fe4ba3f..3acbe9d 100644
> > --- a/include/linux/audit.h
> > +++ b/include/linux/audit.h
> > @@ -154,6 +154,8 @@ extern void
> > audit_log_link_denied(const char *operation, extern int
> > audit_log_task_context(struct audit_buffer *ab); extern void
> > audit_log_task_info(struct audit_buffer *ab, struct task_struct *tsk);
> > +extern int audit_log_container_info(struct task_struct *tsk,
> > + struct audit_context *context);
> >
> > extern int audit_update_lsm_rules(void);
> >
> > @@ -205,6 +207,9 @@ static inline int audit_log_task_context(struct
> > audit_buffer *ab) static inline void audit_log_task_info(struct
> > audit_buffer *ab, struct task_struct *tsk)
> > { }
> > +static inline int audit_log_container_info(struct task_struct *tsk,
> > + struct audit_context
> > *context); +{ }
> > #define audit_enabled 0
> > #endif /* CONFIG_AUDIT */
> >
> > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> > index 921a71f..e83ccbd 100644
> > --- a/include/uapi/linux/audit.h
> > +++ b/include/uapi/linux/audit.h
> > @@ -115,6 +115,7 @@
> > #define AUDIT_REPLACE 1329 /* Replace auditd
> > if this packet unanswerd */ #define AUDIT_KERN_MODULE
> > 1330 /* Kernel Module events */ #define
> > AUDIT_FANOTIFY 1331 /* Fanotify access decision
> > */ +#define AUDIT_CONTAINER_INFO 1332 /* Container ID
> > information */ #define AUDIT_AVC 1400 /* SE
> > Linux avc denial or grant */ #define AUDIT_SELINUX_ERR
> > 1401 /* Internal SE Linux Errors */ diff --git
> > a/kernel/audit.c b/kernel/audit.c index 3f2f143..a12f21f 100644
> > --- a/kernel/audit.c
> > +++ b/kernel/audit.c
> > @@ -2049,6 +2049,26 @@ void audit_log_session_info(struct
> > audit_buffer *ab) audit_log_format(ab, " auid=%u ses=%u", auid,
> > sessionid); }
> >
> > +/*
> > + * audit_log_container_info - report container info
> > + * @tsk: task to be recorded
> > + * @context: task or local context for record
> > + */
> > +int audit_log_container_info(struct task_struct *tsk, struct
> > audit_context *context) +{
> > + struct audit_buffer *ab;
> > +
> > + if (!audit_containerid_set(tsk))
> > + return 0;
> > + /* Generate AUDIT_CONTAINER_INFO with container ID */
> > + ab = audit_log_start(context, GFP_KERNEL,
> > AUDIT_CONTAINER_INFO);
> > + if (!ab)
> > + return -ENOMEM;
> > + audit_log_format(ab, "contid=%llu",
> > audit_get_containerid(tsk));
> > + audit_log_end(ab);
> > + return 0;
> > +}
> > +
> > void audit_log_key(struct audit_buffer *ab, char *key)
> > {
> > audit_log_format(ab, " key=");
> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > index a6b0a52..65be110 100644
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -1453,6 +1453,8 @@ static void audit_log_exit(struct audit_context
> > *context, struct task_struct *ts
> > audit_log_proctitle(tsk, context);
> >
> > + audit_log_container_info(tsk, context);
>
> Would there be any problem moving audit_log_container_info before
> audit_log_proctitle? There are some assumptions that proctitle is the
> last record in some situations.
I see no problem doing that.
> Thanks,
> -Steve
>
> > /* Send end of event record to help user space know we are
> > finished */ ab = audit_log_start(context, GFP_KERNEL, AUDIT_EOE);
> > if (ab)
- RGB
--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
^ permalink raw reply
* Re: [RFC PATCH ghak32 V2 03/13] audit: log container info of syscalls
From: Steve Grubb @ 2018-05-17 21:09 UTC (permalink / raw)
To: Richard Guy Briggs
Cc: cgroups, containers, linux-api, Linux-Audit Mailing List,
linux-fsdevel, LKML, netdev, ebiederm, luto, jlayton, carlos,
dhowells, viro, simo, eparis, serge
In-Reply-To: <6768d20c636df65534f8d325529669bb30a58382.1521179281.git.rgb@redhat.com>
On Fri, 16 Mar 2018 05:00:30 -0400
Richard Guy Briggs <rgb@redhat.com> wrote:
> Create a new audit record AUDIT_CONTAINER_INFO to document the
> container ID of a process if it is present.
As mentioned in a previous email, I think AUDIT_CONTAINER is more
suitable for the container record. One more comment below...
> Called from audit_log_exit(), syscalls are covered.
>
> A sample raw event:
> type=SYSCALL msg=audit(1519924845.499:257): arch=c000003e syscall=257
> success=yes exit=3 a0=ffffff9c a1=56374e1cef30 a2=241 a3=1b6 items=2
> ppid=606 pid=635 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0
> sgid=0 fsgid=0 tty=pts0 ses=3 comm="bash" exe="/usr/bin/bash"
> subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
> key="tmpcontainerid" type=CWD msg=audit(1519924845.499:257):
> cwd="/root" type=PATH msg=audit(1519924845.499:257): item=0
> name="/tmp/" inode=13863 dev=00:27 mode=041777 ouid=0 ogid=0
> rdev=00:00 obj=system_u:object_r:tmp_t:s0 nametype= PARENT
> cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
> type=PATH msg=audit(1519924845.499:257): item=1
> name="/tmp/tmpcontainerid" inode=17729 dev=00:27 mode=0100644 ouid=0
> ogid=0 rdev=00:00 obj=unconfined_u:object_r:user_tmp_t:s0
> nametype=CREATE cap_fp=0000000000000000 cap_fi=0000000000000000
> cap_fe=0 cap_fver=0 type=PROCTITLE msg=audit(1519924845.499:257):
> proctitle=62617368002D6300736C65657020313B206563686F2074657374203E202F746D702F746D70636F6E7461696E65726964
> type=CONTAINER_INFO msg=audit(1519924845.499:257): op=task
> contid=123458
>
> See: https://github.com/linux-audit/audit-kernel/issues/32
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
> include/linux/audit.h | 5 +++++
> include/uapi/linux/audit.h | 1 +
> kernel/audit.c | 20 ++++++++++++++++++++
> kernel/auditsc.c | 2 ++
> 4 files changed, 28 insertions(+)
>
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index fe4ba3f..3acbe9d 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -154,6 +154,8 @@ extern void
> audit_log_link_denied(const char *operation, extern int
> audit_log_task_context(struct audit_buffer *ab); extern void
> audit_log_task_info(struct audit_buffer *ab, struct task_struct *tsk);
> +extern int audit_log_container_info(struct task_struct *tsk,
> + struct audit_context *context);
>
> extern int audit_update_lsm_rules(void);
>
> @@ -205,6 +207,9 @@ static inline int audit_log_task_context(struct
> audit_buffer *ab) static inline void audit_log_task_info(struct
> audit_buffer *ab, struct task_struct *tsk)
> { }
> +static inline int audit_log_container_info(struct task_struct *tsk,
> + struct audit_context
> *context); +{ }
> #define audit_enabled 0
> #endif /* CONFIG_AUDIT */
>
> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> index 921a71f..e83ccbd 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -115,6 +115,7 @@
> #define AUDIT_REPLACE 1329 /* Replace auditd
> if this packet unanswerd */ #define AUDIT_KERN_MODULE
> 1330 /* Kernel Module events */ #define
> AUDIT_FANOTIFY 1331 /* Fanotify access decision
> */ +#define AUDIT_CONTAINER_INFO 1332 /* Container ID
> information */ #define AUDIT_AVC 1400 /* SE
> Linux avc denial or grant */ #define AUDIT_SELINUX_ERR
> 1401 /* Internal SE Linux Errors */ diff --git
> a/kernel/audit.c b/kernel/audit.c index 3f2f143..a12f21f 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -2049,6 +2049,26 @@ void audit_log_session_info(struct
> audit_buffer *ab) audit_log_format(ab, " auid=%u ses=%u", auid,
> sessionid); }
>
> +/*
> + * audit_log_container_info - report container info
> + * @tsk: task to be recorded
> + * @context: task or local context for record
> + */
> +int audit_log_container_info(struct task_struct *tsk, struct
> audit_context *context) +{
> + struct audit_buffer *ab;
> +
> + if (!audit_containerid_set(tsk))
> + return 0;
> + /* Generate AUDIT_CONTAINER_INFO with container ID */
> + ab = audit_log_start(context, GFP_KERNEL,
> AUDIT_CONTAINER_INFO);
> + if (!ab)
> + return -ENOMEM;
> + audit_log_format(ab, "contid=%llu",
> audit_get_containerid(tsk));
> + audit_log_end(ab);
> + return 0;
> +}
> +
> void audit_log_key(struct audit_buffer *ab, char *key)
> {
> audit_log_format(ab, " key=");
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index a6b0a52..65be110 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1453,6 +1453,8 @@ static void audit_log_exit(struct audit_context
> *context, struct task_struct *ts
> audit_log_proctitle(tsk, context);
>
> + audit_log_container_info(tsk, context);
Would there be any problem moving audit_log_container_info before
audit_log_proctitle? There are some assumptions that proctitle is the
last record in some situations.
Thanks,
-Steve
> /* Send end of event record to help user space know we are
> finished */ ab = audit_log_start(context, GFP_KERNEL, AUDIT_EOE);
> if (ab)
^ permalink raw reply
* Re: [RFC PATCH ghak32 V2 01/13] audit: add container id
From: Steve Grubb @ 2018-05-17 21:00 UTC (permalink / raw)
To: Richard Guy Briggs
Cc: simo-H+wXaHxf7aLQT0dZR+AlfA, jlayton-H+wXaHxf7aLQT0dZR+AlfA,
carlos-H+wXaHxf7aLQT0dZR+AlfA, linux-api-u79uwXL29TY76Z2rM5mHXA,
containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, LKML,
eparis-FjpueFixGhCM4zKIHC2jIg, dhowells-H+wXaHxf7aLQT0dZR+AlfA,
Linux-Audit Mailing List, ebiederm-aS9lmoZGLiVWk0Htik3J/w,
luto-DgEjT+Ai2ygdnm+yROfE0A, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
cgroups-u79uwXL29TY76Z2rM5mHXA,
viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn
In-Reply-To: <e284617ad667ad8f17958dd8babb87fe1b4d7205.1521179281.git.rgb-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Fri, 16 Mar 2018 05:00:28 -0400
Richard Guy Briggs <rgb-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> Implement the proc fs write to set the audit container ID of a
> process, emitting an AUDIT_CONTAINER record to document the event.
>
> This is a write from the container orchestrator task to a proc entry
> of the form /proc/PID/containerid where PID is the process ID of the
> newly created task that is to become the first task in a container,
> or an additional task added to a container.
>
> The write expects up to a u64 value (unset: 18446744073709551615).
>
> This will produce a record such as this:
> type=CONTAINER msg=audit(1519903238.968:261): op=set pid=596 uid=0
> subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 auid=0
> tty=pts0 ses=1 opid=596 old-contid=18446744073709551615 contid=123455
> res=0
The was one thing I was wondering about. Currently when we set the
loginuid, the record is AUDIT_LOGINUID. The corollary is that when we
set the container id, the event should be AUDIT_CONTAINERID or
AUDIT_CONTAINER_ID.
During syscall events, the path info is returned in a a record simply
called AUDIT_PATH, cwd info is returned in AUDIT_CWD. So, rather than
calling the record that gets attached to everything
AUDIT_CONTAINER_INFO, how about simply AUDIT_CONTAINER.
> The "op" field indicates an initial set. The "pid" to "ses" fields
> are the orchestrator while the "opid" field is the object's PID, the
> process being "contained". Old and new container ID values are given
> in the "contid" fields, while res indicates its success.
>
> It is not permitted to self-set, unset or re-set the container ID. A
> child inherits its parent's container ID, but then can be set only
> once after.
>
> See: https://github.com/linux-audit/audit-kernel/issues/32
>
> Signed-off-by: Richard Guy Briggs <rgb-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> fs/proc/base.c | 37 ++++++++++++++++++++
> include/linux/audit.h | 16 +++++++++
> include/linux/init_task.h | 4 ++-
> include/linux/sched.h | 1 +
> include/uapi/linux/audit.h | 2 ++
> kernel/auditsc.c | 84
> ++++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 143
> insertions(+), 1 deletion(-)
>
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index 60316b5..6ce4fbe 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -1299,6 +1299,41 @@ static ssize_t proc_sessionid_read(struct file
> * file, char __user * buf, .read = proc_sessionid_read,
> .llseek = generic_file_llseek,
> };
> +
> +static ssize_t proc_containerid_write(struct file *file, const char
> __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + struct inode *inode = file_inode(file);
> + u64 containerid;
> + int rv;
> + struct task_struct *task = get_proc_task(inode);
> +
> + if (!task)
> + return -ESRCH;
> + if (*ppos != 0) {
> + /* No partial writes. */
> + put_task_struct(task);
> + return -EINVAL;
> + }
> +
> + rv = kstrtou64_from_user(buf, count, 10, &containerid);
> + if (rv < 0) {
> + put_task_struct(task);
> + return rv;
> + }
> +
> + rv = audit_set_containerid(task, containerid);
> + put_task_struct(task);
> + if (rv < 0)
> + return rv;
> + return count;
> +}
> +
> +static const struct file_operations proc_containerid_operations = {
> + .write = proc_containerid_write,
> + .llseek = generic_file_llseek,
> +};
> +
> #endif
>
> #ifdef CONFIG_FAULT_INJECTION
> @@ -2961,6 +2996,7 @@ static int proc_pid_patch_state(struct seq_file
> *m, struct pid_namespace *ns, #ifdef CONFIG_AUDITSYSCALL
> REG("loginuid", S_IWUSR|S_IRUGO, proc_loginuid_operations),
> REG("sessionid", S_IRUGO, proc_sessionid_operations),
> + REG("containerid", S_IWUSR, proc_containerid_operations),
> #endif
> #ifdef CONFIG_FAULT_INJECTION
> REG("make-it-fail", S_IRUGO|S_IWUSR,
> proc_fault_inject_operations), @@ -3355,6 +3391,7 @@ static int
> proc_tid_comm_permission(struct inode *inode, int mask) #ifdef
> CONFIG_AUDITSYSCALL REG("loginuid", S_IWUSR|S_IRUGO,
> proc_loginuid_operations), REG("sessionid", S_IRUGO,
> proc_sessionid_operations),
> + REG("containerid", S_IWUSR, proc_containerid_operations),
> #endif
> #ifdef CONFIG_FAULT_INJECTION
> REG("make-it-fail", S_IRUGO|S_IWUSR,
> proc_fault_inject_operations), diff --git a/include/linux/audit.h
> b/include/linux/audit.h index af410d9..fe4ba3f 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -29,6 +29,7 @@
>
> #define AUDIT_INO_UNSET ((unsigned long)-1)
> #define AUDIT_DEV_UNSET ((dev_t)-1)
> +#define INVALID_CID AUDIT_CID_UNSET
>
> struct audit_sig_info {
> uid_t uid;
> @@ -321,6 +322,7 @@ static inline void audit_ptrace(struct
> task_struct *t) extern int auditsc_get_stamp(struct audit_context
> *ctx, struct timespec64 *t, unsigned int *serial);
> extern int audit_set_loginuid(kuid_t loginuid);
> +extern int audit_set_containerid(struct task_struct *tsk, u64
> containerid);
> static inline kuid_t audit_get_loginuid(struct task_struct *tsk)
> {
> @@ -332,6 +334,11 @@ static inline unsigned int
> audit_get_sessionid(struct task_struct *tsk) return tsk->sessionid;
> }
>
> +static inline u64 audit_get_containerid(struct task_struct *tsk)
> +{
> + return tsk->containerid;
> +}
> +
> extern void __audit_ipc_obj(struct kern_ipc_perm *ipcp);
> extern void __audit_ipc_set_perm(unsigned long qbytes, uid_t uid,
> gid_t gid, umode_t mode); extern void __audit_bprm(struct
> linux_binprm *bprm); @@ -517,6 +524,10 @@ static inline unsigned int
> audit_get_sessionid(struct task_struct *tsk) {
> return -1;
> }
> +static inline kuid_t audit_get_containerid(struct task_struct *tsk)
> +{
> + return INVALID_CID;
> +}
> static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp)
> { }
> static inline void audit_ipc_set_perm(unsigned long qbytes, uid_t
> uid, @@ -581,6 +592,11 @@ static inline bool
> audit_loginuid_set(struct task_struct *tsk) return
> uid_valid(audit_get_loginuid(tsk)); }
>
> +static inline bool audit_containerid_set(struct task_struct *tsk)
> +{
> + return audit_get_containerid(tsk) != INVALID_CID;
> +}
> +
> static inline void audit_log_string(struct audit_buffer *ab, const
> char *buf) {
> audit_log_n_string(ab, buf, strlen(buf));
> diff --git a/include/linux/init_task.h b/include/linux/init_task.h
> index 6a53262..046bd0a 100644
> --- a/include/linux/init_task.h
> +++ b/include/linux/init_task.h
> @@ -18,6 +18,7 @@
> #include <linux/sched/rt.h>
> #include <linux/livepatch.h>
> #include <linux/mm_types.h>
> +#include <linux/audit.h>
>
> #include <asm/thread_info.h>
>
> @@ -120,7 +121,8 @@
> #ifdef CONFIG_AUDITSYSCALL
> #define INIT_IDS \
> .loginuid = INVALID_UID, \
> - .sessionid = (unsigned int)-1,
> + .sessionid = (unsigned int)-1, \
> + .containerid = INVALID_CID,
> #else
> #define INIT_IDS
> #endif
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index d258826..1b82191 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -796,6 +796,7 @@ struct task_struct {
> #ifdef CONFIG_AUDITSYSCALL
> kuid_t loginuid;
> unsigned int sessionid;
> + u64 containerid;
> #endif
> struct seccomp seccomp;
>
> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> index 4e61a9e..921a71f 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -71,6 +71,7 @@
> #define AUDIT_TTY_SET 1017 /* Set TTY auditing
> status */ #define AUDIT_SET_FEATURE 1018 /* Turn an
> audit feature on or off */ #define AUDIT_GET_FEATURE
> 1019 /* Get which features are enabled */ +#define
> AUDIT_CONTAINER 1020 /* Define the container id
> and information */ #define AUDIT_FIRST_USER_MSG 1100 /*
> Userspace messages mostly uninteresting to kernel */ #define
> AUDIT_USER_AVC 1107 /* We filter this
> differently */ @@ -465,6 +466,7 @@ struct audit_tty_status { };
>
> #define AUDIT_UID_UNSET (unsigned int)-1
> +#define AUDIT_CID_UNSET ((u64)-1)
>
> /* audit_rule_data supports filter rules with both integer and string
> * fields. It corresponds with AUDIT_ADD_RULE, AUDIT_DEL_RULE and
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 4e0a4ac..29c8482 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -2073,6 +2073,90 @@ int audit_set_loginuid(kuid_t loginuid)
> return rc;
> }
>
> +static int audit_set_containerid_perm(struct task_struct *task, u64
> containerid) +{
> + struct task_struct *parent;
> + u64 pcontainerid, ccontainerid;
> +
> + /* Don't allow to set our own containerid */
> + if (current == task)
> + return -EPERM;
> + /* Don't allow the containerid to be unset */
> + if (!cid_valid(containerid))
> + return -EINVAL;
> + /* if we don't have caps, reject */
> + if (!capable(CAP_AUDIT_CONTROL))
> + return -EPERM;
> + /* if containerid is unset, allow */
> + if (!audit_containerid_set(task))
> + return 0;
> + /* it is already set, and not inherited from the parent,
> reject */
> + ccontainerid = audit_get_containerid(task);
> + rcu_read_lock();
> + parent = rcu_dereference(task->real_parent);
> + rcu_read_unlock();
> + task_lock(parent);
> + pcontainerid = audit_get_containerid(parent);
> + task_unlock(parent);
> + if (ccontainerid != pcontainerid)
> + return -EPERM;
> + return 0;
> +}
> +
> +static void audit_log_set_containerid(struct task_struct *task, u64
> oldcontainerid,
> + u64 containerid, int rc)
> +{
> + struct audit_buffer *ab;
> + uid_t uid;
> + struct tty_struct *tty;
> +
> + if (!audit_enabled)
> + return;
> +
> + ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONTAINER);
> + if (!ab)
> + return;
> +
> + uid = from_kuid(&init_user_ns, task_uid(current));
> + tty = audit_get_tty(current);
> +
> + audit_log_format(ab, "op=set pid=%d uid=%u",
> task_tgid_nr(current), uid);
> + audit_log_task_context(ab);
> + audit_log_format(ab, " auid=%u tty=%s ses=%u opid=%d
> old-contid=%llu contid=%llu res=%d",
The preferred ordering would be: op, opid, old-contid, contid, pid, uid,
tty, ses, subj, comm, exe, res. This groups the searchable fields
together using the most common ordering so that parsing is simple.
Thanks,
-Steve
> + from_kuid(&init_user_ns,
> audit_get_loginuid(current)),
> + tty ? tty_name(tty) : "(none)",
> audit_get_sessionid(current),
> + task_tgid_nr(task), oldcontainerid,
> containerid, !rc); +
> + audit_put_tty(tty);
> + audit_log_end(ab);
> +}
> +
> +/**
> + * audit_set_containerid - set current task's audit_context
> containerid
> + * @containerid: containerid value
> + *
> + * Returns 0 on success, -EPERM on permission failure.
> + *
> + * Called (set) from fs/proc/base.c::proc_containerid_write().
> + */
> +int audit_set_containerid(struct task_struct *task, u64 containerid)
> +{
> + u64 oldcontainerid;
> + int rc;
> +
> + oldcontainerid = audit_get_containerid(task);
> +
> + rc = audit_set_containerid_perm(task, containerid);
> + if (!rc) {
> + task_lock(task);
> + task->containerid = containerid;
> + task_unlock(task);
> + }
> +
> + audit_log_set_containerid(task, oldcontainerid, containerid,
> rc);
> + return rc;
> +}
> +
> /**
> * __audit_mq_open - record audit data for a POSIX MQ open
> * @oflag: open flag
^ permalink raw reply
* [PATCH v4 3/3] fs: Add aio iopriority support for block_dev
From: adam.manzanares @ 2018-05-17 20:38 UTC (permalink / raw)
To: viro, linux-fsdevel, axboe, bcrl
Cc: mingo, peterz, pombredanne, gregkh, bigeasy, rgoldwyn,
linux-block, linux-kernel, linux-aio, linux-api, Adam Manzanares
In-Reply-To: <20180517203803.2664-1-adam.manzanares@wdc.com>
From: Adam Manzanares <adam.manzanares@wdc.com>
This is the per-I/O equivalent of the ioprio_set system call.
When IOCB_FLAG_IOPRIO is set on the iocb aio_flags field, then we set the
newly added kiocb ki_ioprio field to the value in the iocb aio_reqprio field.
When a bio is created for an aio request by the block dev we set the priority
value of the bio to the user supplied value.
This patch depends on block: add ioprio_check_cap function
Signed-off-by: Adam Manzanares <adam.manzanares@wdc.com>
---
fs/aio.c | 16 ++++++++++++++++
fs/block_dev.c | 2 ++
include/linux/fs.h | 2 ++
include/uapi/linux/aio_abi.h | 1 +
4 files changed, 21 insertions(+)
diff --git a/fs/aio.c b/fs/aio.c
index f3eae5d5771b..ff3107aa82d5 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -1451,6 +1451,22 @@ static int aio_prep_rw(struct kiocb *req, struct iocb *iocb)
if (iocb->aio_flags & IOCB_FLAG_RESFD)
req->ki_flags |= IOCB_EVENTFD;
req->ki_hint = file_write_hint(req->ki_filp);
+ if (iocb->aio_flags & IOCB_FLAG_IOPRIO) {
+ /*
+ * If the IOCB_FLAG_IOPRIO flag of aio_flags is set, then
+ * aio_reqprio is interpreted as an I/O scheduling
+ * class and priority.
+ */
+ ret = ioprio_check_cap(iocb->aio_reqprio);
+ if (ret) {
+ pr_debug("aio ioprio check cap error\n");
+ return -EINVAL;
+ }
+
+ req->ki_ioprio = iocb->aio_reqprio;
+ req->ki_flags |= IOCB_IOPRIO;
+ }
+
ret = kiocb_set_rw_flags(req, iocb->aio_rw_flags);
if (unlikely(ret))
fput(req->ki_filp);
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 7ec920e27065..970bef79caa6 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -355,6 +355,8 @@ __blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter, int nr_pages)
bio->bi_write_hint = iocb->ki_hint;
bio->bi_private = dio;
bio->bi_end_io = blkdev_bio_end_io;
+ if (iocb->ki_flags & IOCB_IOPRIO)
+ bio->bi_ioprio = iocb->ki_ioprio;
ret = bio_iov_iter_get_pages(bio, iter);
if (unlikely(ret)) {
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 7a90ce387e00..3415e83f6350 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -294,6 +294,7 @@ enum rw_hint {
#define IOCB_SYNC (1 << 5)
#define IOCB_WRITE (1 << 6)
#define IOCB_NOWAIT (1 << 7)
+#define IOCB_IOPRIO (1 << 8)
struct kiocb {
struct file *ki_filp;
@@ -302,6 +303,7 @@ struct kiocb {
void *private;
int ki_flags;
u16 ki_hint;
+ u16 ki_ioprio; /* See linux/ioprio.h */
} __randomize_layout;
static inline bool is_sync_kiocb(struct kiocb *kiocb)
diff --git a/include/uapi/linux/aio_abi.h b/include/uapi/linux/aio_abi.h
index 2c0a3415beee..d4e768d55d14 100644
--- a/include/uapi/linux/aio_abi.h
+++ b/include/uapi/linux/aio_abi.h
@@ -55,6 +55,7 @@ enum {
* is valid.
*/
#define IOCB_FLAG_RESFD (1 << 0)
+#define IOCB_FLAG_IOPRIO (1 << 1)
/* read() from /dev/aio returns these structures. */
struct io_event {
--
2.15.1
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox