Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH resend v2 0/3] virt: Add vboxguest driver for Virtual Box
 Guest integration
To:     Hans de Goede <hdegoede@redhat.com>, Arnd Bergmann <arnd@arndb.de>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc:     Michael Thayer <michael.thayer@oracle.com>,
        "Knut St . Osmundsen" <knut.osmundsen@oracle.com>,
        Christoph Hellwig <hch@infradead.org>,
        linux-kernel@vger.kernel.org
References: <20171126151221.26410-1-hdegoede@redhat.com>
 <ef210f2c-58b0-807f-56a7-e55371730371@lwfinger.net>
 <b2483716-17ff-4ffd-87e2-b8ff89cd3fdc@redhat.com>
From:   Larry Finger <Larry.Finger@lwfinger.net>
Message-ID: <7f5d966c-8ad9-810b-7b6e-fe8ac0c698ff@lwfinger.net>
Date:   Tue, 28 Nov 2017 10:15:09 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.4.0
MIME-Version: 1.0
In-Reply-To: <b2483716-17ff-4ffd-87e2-b8ff89cd3fdc@redhat.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 11/28/2017 04:01 AM, Hans de Goede wrote:
> Hi,
> 

>> I did have two problems when I tried to build these commits and the one that 
>> creates vboxsf.
>>
>> The more serious one is that it is possible to build vboxguest without 
>> vboxvideo. When that happens, a non-privileged user cannot start X. As I say 
>> in the review, > I think that combination does not make sense and should not 
>> be allowed.
> 
> vboxguest and vboxvideo are completely independent at least from the kernel pov,
> I do not believe that making them depend on each other makes sense.
> 
> AFAIK a non-privileged user cannot start X without vboxvideo at all, independent
> of vboxguest being build or not. Falling back to vesa modesetting always requires
> Xorg to be suid root, or the user to be privileged.
> 
> TL;DR: I can add a dependency between the 2, but I would rather not.

Keep in mind that at some point, the newest kernel will support vboxvideo and 
vboxguest; however, any distribution package will still need to contain both 
kernel modules so that older kernels will work. My test showed that loading an 
in-kernel vboxguest with Oracle's vboxvideo fails *unless* you run as root, 
which is not acceptable. When both are in the kernel, then Xorg starts for a 
non-privaleged user. That is why I think you need either a "depends on 
VBOXVIDEO" or a "selects VBOXVIDEO" in the Kconfig for vboxvideo. My preference 
is for the latter.

>> When the system is booted, vboxsf is not loaded, and the shared folders are 
>> not automounted. Of course, that issue is not germane to these patches, but 
>> will be important when vboxsf is merged.
> 
> Hmm, I mount a couple of shares from rc.local (I don't use vbox' automount as I
> want to specify a uid for the files) and as soon as mount.vboxsf gets executed
> the vboxsf module gets auto-loaded as the module contains:
> 
> MODULE_ALIAS_FS("vboxsf");
> 
> AFAIK the communication of which volumes to automount is done through vboxguest,
> anyways I will look into this before submitting vboxsf, in the worst case
> we need to drop a modprobe.conf.d/vboxguest.conf file which has a postinst 
> vboxguest
> which loads vboxsf.

Thanks. Adding something of this type will make the in-kernel version match the 
Oracle documentation.

Larry

From 1585303649081607577@xxx Tue Nov 28 10:02:53 +0000 2017
X-GM-THRID: 1585142010203838290
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread