Date: Sun, 19 Mar 2017 22:12:22 +0100 From: Gergely Czuczy <gergely.czuczy@harmless.hu> To: Rick Macklem <rmacklem@uoguelph.ca>, Konstantin Belousov <kostikbel@gmail.com> Cc: Dimitry Andric <dim@FreeBSD.org>, Ian Lepore <ian@FreeBSD.org>, FreeBSD Current <freebsd-current@freebsd.org> Subject: Re: process killed: text file modification Message-ID: <4642046a-08e6-35af-c76e-c5e306f01e62@harmless.hu> In-Reply-To: <YTXPR01MB0189F47B6A23C10BFE8A85E6DD3B0@YTXPR01MB0189.CANPRD01.PROD.OUTLOOK.COM> References: <5ac94b9a-7ced-9eff-d746-7dddaaeca516@harmless.hu> <1489340839.40576.82.camel@freebsd.org> <FF55DB37-4A6B-4D88-B201-B3BCA1A11E87@FreeBSD.org> <YTXPR01MB01898D49933A82170FCAB7A0DD390@YTXPR01MB0189.CANPRD01.PROD.OUTLOOK.COM> <YTXPR01MB018944EF4248402AD421D825DD390@YTXPR01MB0189.CANPRD01.PROD.OUTLOOK.COM> <20170317083605.GQ16105@kib.kiev.ua> <YTXPR01MB0189F7147A7C5C5F8C56B2F1DD390@YTXPR01MB0189.CANPRD01.PROD.OUTLOOK.COM> <20170317141917.GS16105@kib.kiev.ua> <D0770019-3EEA-45D2-A751-18DF1B274F90@FreeBSD.org> <YTXPR01MB0189FBB6CF664653C1162936DD390@YTXPR01MB0189.CANPRD01.PROD.OUTLOOK.COM> <20170318032150.GW16105@kib.kiev.ua> <YTXPR01MB0189F47B6A23C10BFE8A85E6DD3B0@YTXPR01MB0189.CANPRD01.PROD.OUTLOOK.COM>
next in thread | previous in thread | raw e-mail | index | archive | help
On 2017. 03. 19. 21:52, Rick Macklem wrote:
> Kostik wrote:
> [stuff snipped]
>>>>> Dirty pages are flushed by writes, so if we have a set of dirty pages and
>>>>> async vm_object_page_clean() is called on the vnode' vm_object, we get
>>>>> a bunch of delayed-write AKA dirty buffers. This is possible even after
>>>>> VOP_CLOSE() was done, e.g. by syncer performing regular run involving
>>>>> vfs_msync().
>>> When I was talking about ncl_flush() above, I was referring to buffer cache
>>> buffers written by a write(2) syscall, not the case of mmap'd pages.
>> But dirty buffers can appear on the vnode queue due to dirty pages msyncing
>> by syncer, for instance.
> Ok, just to clarify this, in case I don't understand it...
> - You aren't saying that anything will be added to v_bufobj.bo_dirty.bv_hd by
> vfs_msync() or similar, after VOP_CLOSE(), right?
> --> ncl_flush() { was called nfs_flush() in the old NFS client } only deals with
> "struct buf's" hanging off v_bufobj.bo_dirty.bv_hd, so I don't see a use for
> it in the patch.
>
> As for pages added to v_bufobj.bo_object...the patch assumes that the process
> that was writing the executable file mmap'd is done { normally exited } before
> the exec() syscall occurs. If it is still dirtying pages when the exec() occurs, then
> failing with "Text file modified" seems correct to me. As you mentioned, another
> client can do this to the file anyhow.
>
> My understanding is that vm_object_page_clean() will get all the dirty pages written
> back to the server at that point and if that is done in VOP_SET_TEXT() as this patch
> does, what more can the NFS client do?
>
> [more stuff snipped]
>> Syncer does not open the vnode inside the vfs_msync() operations.
> Ok, but this doesn't put "struct buf's" on v_bufobj.bo_dirty.bv_hd. Am I right?
> (When I said "buffers". I meant "struct buf's" under bo_dirty, not stuff under
> v_bufobj.bo_object.)
>
>> We do track writeability to the file, and do not allow execution if there is
>> an active writer, be it a file descriptor opened for write, or a writeable
>> mapping. And in reverse, if the file is executed (VV_TEXT is set), then
>> we disallow opening the file for write.
> Yes, and that was why I figured doing this in VOP_SET_TEXT(), just before
> setting VV_TEXT, was the right place to do it.
> [more stuff snipped]
>> Thanks for testing the patch. Now, if others can test it...rick
>>
> Again, hopefully others (especially the original reporter) will be able to
> test the patch, rick
Actually I want to test it, but you guys are so vehemently discussing
it, I thought it would be better to do so, once you guys settled your
analysis on the code. Also, me not having the problem occurring, I don't
think would mean it's solved, since that would only mean, the codepath
for my specific usecase works. There might be other things there as
well, what I don't hit.
Let me know which patch should I test, and I will see to it in the next
couple of days, when I get the time to do it.
Regards,
-czg
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4642046a-08e6-35af-c76e-c5e306f01e62>