netdev - Re: [PATCH v3 6/7] 9p/trans_virtio: support larger msize values

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

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

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

Message-ID: <1797352.eH9cFvQebf@silver>
Date:   Mon, 22 Nov 2021 14:32:23 +0100
From:   Christian Schoenebeck <linux_oss@...debyte.com>
To:     Dominique Martinet <asmadeus@...ewreck.org>
Cc:     Nikolay Kichukov <nikolay@...um.net>,
        v9fs-developer@...ts.sourceforge.net, netdev@...r.kernel.org,
        Eric Van Hensbergen <ericvh@...il.com>,
        Latchesar Ionkov <lucho@...kov.net>,
        Greg Kurz <groug@...d.org>, Vivek Goyal <vgoyal@...hat.com>
Subject: Re: [PATCH v3 6/7] 9p/trans_virtio: support larger msize values

On Sonntag, 21. November 2021 23:12:14 CET Dominique Martinet wrote:
> Christian Schoenebeck wrote on Sun, Nov 21, 2021 at 05:57:30PM +0100:
> > > Although frankly as I said if we're going to do this, we actual can
> > > majorate the actual max for all operations pretty easily thanks to the
> > > count parameter -- I guess it's a bit more work but we can put arbitrary
> > > values (e.g. 8k for all the small stuff) instead of trying to figure it
> > > out more precisely; I'd just like the code path to be able to do it so
> > > we only do that rechurn once.
> > 
> > Looks like we had a similar idea on this. My plan was something like this:
> > 
> > static int max_msg_size(enum msg_type) {
> > 
> >     switch (msg_type) {
> >     
> >         /* large zero copy messages */
> >         case Twrite:
> >         case Tread:
> >         
> >         case Treaddir:
> >             BUG_ON(true);
> >         
> >         /* small messages */
> >         case Tversion:
> >         ....
> >         
> >             return 8k; /* to be replaced with appropriate max value */
> >     
> >     }
> > 
> > }
> > 
> > That would be a quick start and allow to fine grade in future. It would
> > also provide a safety net, e.g. the compiler would bark if a new message
> > type is added in future.
> 
> I assume that'd only be used if the caller does not set an explicit
> limit, at which point we're basically using a constant and the function
> coud be replaced by a P9_SMALL_MSG_SIZE constant... But yes, I agree
> with the idea, it's these three calls that deal with big buffers in
> either emission or reception (might as well not allocate a 128MB send
> buffer for Tread ;))

I "think" this could be used for all 9p message types except for the listed 
former three, but I'll review the 9p specs more carefully before v4. For Tread 
and Twrite we already received the requested size, which just leaves Treaddir, 
which is however indeed tricky, because I don't think we have any info how 
many directory entries we could expect.

A simple compile time constant (e.g. one macro) could be used instead of this 
function. If you prefer a constant instead, I could go for it in v4 of course. 
For this 9p client I would recommend a function though, simply because this 
code has already seen some authors come and go over the years, so it might be 
worth the redundant code for future safety. But I'll adapt to what others 
think.

Greg, opinion?

Best regards,
Christian Schoenebeck