[PATCH 4.19 064/267] crypto: virtio: Fix use-after-free in virtio_crypto_skcipher_finalize_req()
From: Longpeng(Mike) [ Upstream commit 8c855f0720ff006d75d0a2512c7f6c4f60ff60ee ] The system'll crash when the users insmod crypto/tcrypto.ko with mode=155 ( testing "authenc(hmac(sha1),cbc(aes))" ). It's caused by reuse the memory of request structure. In crypto_authenc_init_tfm(), the reqsize is set to: [PART 1] sizeof(authenc_request_ctx) + [PART 2] ictx->reqoff + [PART 3] MAX(ahash part, skcipher part) and the 'PART 3' is used by both ahash and skcipher in turn. When the virtio_crypto driver finish skcipher req, it'll call ->complete callback(in crypto_finalize_skcipher_request) and then free its resources whose pointers are recorded in 'skcipher parts'. However, the ->complete is 'crypto_authenc_encrypt_done' in this case, it will use the 'ahash part' of the request and change its content, so virtio_crypto driver will get the wrong pointer after ->complete finish and mistakenly free some other's memory. So the system will crash when these memory will be used again. The resources which need to be cleaned up are not used any more. But the pointers of these resources may be changed in the function "crypto_finalize_skcipher_request". Thus release specific resources before calling this function. Fixes: dbaf0624ffa5 ("crypto: add virtio-crypto driver") Reported-by: LABBE Corentin Cc: Gonglei Cc: Herbert Xu Cc: "Michael S. Tsirkin" Cc: Jason Wang Cc: "David S. Miller" Cc: virtualization@lists.linux-foundation.org Cc: linux-ker...@vger.kernel.org Cc: sta...@vger.kernel.org Link: https://lore.kernel.org/r/20200123101000.GB24255@Red Acked-by: Gonglei Signed-off-by: Longpeng(Mike) Link: https://lore.kernel.org/r/20200602070501.2023-3-longpe...@huawei.com Signed-off-by: Michael S. Tsirkin Signed-off-by: Sasha Levin --- drivers/crypto/virtio/virtio_crypto_algs.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/crypto/virtio/virtio_crypto_algs.c b/drivers/crypto/virtio/virtio_crypto_algs.c index 38432721069f..9348060cc32f 100644 --- a/drivers/crypto/virtio/virtio_crypto_algs.c +++ b/drivers/crypto/virtio/virtio_crypto_algs.c @@ -594,10 +594,11 @@ static void virtio_crypto_ablkcipher_finalize_req( scatterwalk_map_and_copy(req->info, req->dst, req->nbytes - AES_BLOCK_SIZE, AES_BLOCK_SIZE, 0); - crypto_finalize_ablkcipher_request(vc_sym_req->base.dataq->engine, - req, err); kzfree(vc_sym_req->iv); virtcrypto_clear_request(&vc_sym_req->base); + + crypto_finalize_ablkcipher_request(vc_sym_req->base.dataq->engine, + req, err); } static struct virtio_crypto_algo virtio_crypto_algs[] = { { -- 2.25.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 4.19 065/267] crypto: virtio: Fix src/dst scatterlist calculation in __virtio_crypto_skcipher_do_req()
From: Longpeng(Mike) [ Upstream commit b02989f37fc5e865c9070907e4493b3a21e2 ] The system will crash when the users insmod crypto/tcrypt.ko with mode=38 ( testing "cts(cbc(aes))" ). Usually the next entry of one sg will be @sg@ + 1, but if this sg element is part of a chained scatterlist, it could jump to the start of a new scatterlist array. Fix it by sg_next() on calculation of src/dst scatterlist. Fixes: dbaf0624ffa5 ("crypto: add virtio-crypto driver") Reported-by: LABBE Corentin Cc: Herbert Xu Cc: "Michael S. Tsirkin" Cc: Jason Wang Cc: "David S. Miller" Cc: virtualization@lists.linux-foundation.org Cc: linux-ker...@vger.kernel.org Cc: sta...@vger.kernel.org Link: https://lore.kernel.org/r/20200123101000.GB24255@Red Signed-off-by: Gonglei Signed-off-by: Longpeng(Mike) Link: https://lore.kernel.org/r/20200602070501.2023-2-longpe...@huawei.com Signed-off-by: Michael S. Tsirkin Signed-off-by: Sasha Levin --- drivers/crypto/virtio/virtio_crypto_algs.c | 15 ++- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/drivers/crypto/virtio/virtio_crypto_algs.c b/drivers/crypto/virtio/virtio_crypto_algs.c index 9348060cc32f..e9a8485c4929 100644 --- a/drivers/crypto/virtio/virtio_crypto_algs.c +++ b/drivers/crypto/virtio/virtio_crypto_algs.c @@ -367,13 +367,18 @@ __virtio_crypto_ablkcipher_do_req(struct virtio_crypto_sym_request *vc_sym_req, int err; unsigned long flags; struct scatterlist outhdr, iv_sg, status_sg, **sgs; - int i; u64 dst_len; unsigned int num_out = 0, num_in = 0; int sg_total; uint8_t *iv; + struct scatterlist *sg; src_nents = sg_nents_for_len(req->src, req->nbytes); + if (src_nents < 0) { + pr_err("Invalid number of src SG.\n"); + return src_nents; + } + dst_nents = sg_nents(req->dst); pr_debug("virtio_crypto: Number of sgs (src_nents: %d, dst_nents: %d)\n", @@ -459,12 +464,12 @@ __virtio_crypto_ablkcipher_do_req(struct virtio_crypto_sym_request *vc_sym_req, vc_sym_req->iv = iv; /* Source data */ - for (i = 0; i < src_nents; i++) - sgs[num_out++] = &req->src[i]; + for (sg = req->src; src_nents; sg = sg_next(sg), src_nents--) + sgs[num_out++] = sg; /* Destination data */ - for (i = 0; i < dst_nents; i++) - sgs[num_out + num_in++] = &req->dst[i]; + for (sg = req->dst; sg; sg = sg_next(sg)) + sgs[num_out + num_in++] = sg; /* Status */ sg_init_one(&status_sg, &vc_req->status, sizeof(vc_req->status)); -- 2.25.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 4.19 066/267] crypto: virtio: Fix dest length calculation in __virtio_crypto_skcipher_do_req()
From: Longpeng(Mike) [ Upstream commit d90ca42012db2863a9a30b564a2ace6016594bda ] The src/dst length is not aligned with AES_BLOCK_SIZE(which is 16) in some testcases in tcrypto.ko. For example, the src/dst length of one of cts(cbc(aes))'s testcase is 17, the crypto_virtio driver will set @src_data_len=16 but @dst_data_len=17 in this case and get a wrong at then end. SRC: pp pp pp pp pp pp pp pp pp pp pp pp pp pp pp pp pp (17 bytes) EXP: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc pp (17 bytes) DST: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc 00 (pollute the last bytes) (pp: plaintext cc:ciphertext) Fix this issue by limit the length of dest buffer. Fixes: dbaf0624ffa5 ("crypto: add virtio-crypto driver") Cc: Gonglei Cc: Herbert Xu Cc: "Michael S. Tsirkin" Cc: Jason Wang Cc: "David S. Miller" Cc: virtualization@lists.linux-foundation.org Cc: linux-ker...@vger.kernel.org Cc: sta...@vger.kernel.org Signed-off-by: Longpeng(Mike) Link: https://lore.kernel.org/r/20200602070501.2023-4-longpe...@huawei.com Signed-off-by: Michael S. Tsirkin Signed-off-by: Sasha Levin --- drivers/crypto/virtio/virtio_crypto_algs.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/crypto/virtio/virtio_crypto_algs.c b/drivers/crypto/virtio/virtio_crypto_algs.c index e9a8485c4929..ab4700e4b409 100644 --- a/drivers/crypto/virtio/virtio_crypto_algs.c +++ b/drivers/crypto/virtio/virtio_crypto_algs.c @@ -424,6 +424,7 @@ __virtio_crypto_ablkcipher_do_req(struct virtio_crypto_sym_request *vc_sym_req, goto free; } + dst_len = min_t(unsigned int, req->nbytes, dst_len); pr_debug("virtio_crypto: src_len: %u, dst_len: %llu\n", req->nbytes, dst_len); -- 2.25.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 4.14 049/190] crypto: virtio: Fix use-after-free in virtio_crypto_skcipher_finalize_req()
From: Longpeng(Mike) [ Upstream commit 8c855f0720ff006d75d0a2512c7f6c4f60ff60ee ] The system'll crash when the users insmod crypto/tcrypto.ko with mode=155 ( testing "authenc(hmac(sha1),cbc(aes))" ). It's caused by reuse the memory of request structure. In crypto_authenc_init_tfm(), the reqsize is set to: [PART 1] sizeof(authenc_request_ctx) + [PART 2] ictx->reqoff + [PART 3] MAX(ahash part, skcipher part) and the 'PART 3' is used by both ahash and skcipher in turn. When the virtio_crypto driver finish skcipher req, it'll call ->complete callback(in crypto_finalize_skcipher_request) and then free its resources whose pointers are recorded in 'skcipher parts'. However, the ->complete is 'crypto_authenc_encrypt_done' in this case, it will use the 'ahash part' of the request and change its content, so virtio_crypto driver will get the wrong pointer after ->complete finish and mistakenly free some other's memory. So the system will crash when these memory will be used again. The resources which need to be cleaned up are not used any more. But the pointers of these resources may be changed in the function "crypto_finalize_skcipher_request". Thus release specific resources before calling this function. Fixes: dbaf0624ffa5 ("crypto: add virtio-crypto driver") Reported-by: LABBE Corentin Cc: Gonglei Cc: Herbert Xu Cc: "Michael S. Tsirkin" Cc: Jason Wang Cc: "David S. Miller" Cc: virtualization@lists.linux-foundation.org Cc: linux-ker...@vger.kernel.org Cc: sta...@vger.kernel.org Link: https://lore.kernel.org/r/20200123101000.GB24255@Red Acked-by: Gonglei Signed-off-by: Longpeng(Mike) Link: https://lore.kernel.org/r/20200602070501.2023-3-longpe...@huawei.com Signed-off-by: Michael S. Tsirkin Signed-off-by: Sasha Levin --- drivers/crypto/virtio/virtio_crypto_algs.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/crypto/virtio/virtio_crypto_algs.c b/drivers/crypto/virtio/virtio_crypto_algs.c index e2231a1a05a1..772d2b3137c6 100644 --- a/drivers/crypto/virtio/virtio_crypto_algs.c +++ b/drivers/crypto/virtio/virtio_crypto_algs.c @@ -569,10 +569,11 @@ static void virtio_crypto_ablkcipher_finalize_req( struct ablkcipher_request *req, int err) { - crypto_finalize_cipher_request(vc_sym_req->base.dataq->engine, - req, err); kzfree(vc_sym_req->iv); virtcrypto_clear_request(&vc_sym_req->base); + + crypto_finalize_cipher_request(vc_sym_req->base.dataq->engine, + req, err); } static struct crypto_alg virtio_crypto_algs[] = { { -- 2.25.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 4.14 050/190] crypto: virtio: Fix src/dst scatterlist calculation in __virtio_crypto_skcipher_do_req()
From: Longpeng(Mike) [ Upstream commit b02989f37fc5e865c9070907e4493b3a21e2 ] The system will crash when the users insmod crypto/tcrypt.ko with mode=38 ( testing "cts(cbc(aes))" ). Usually the next entry of one sg will be @sg@ + 1, but if this sg element is part of a chained scatterlist, it could jump to the start of a new scatterlist array. Fix it by sg_next() on calculation of src/dst scatterlist. Fixes: dbaf0624ffa5 ("crypto: add virtio-crypto driver") Reported-by: LABBE Corentin Cc: Herbert Xu Cc: "Michael S. Tsirkin" Cc: Jason Wang Cc: "David S. Miller" Cc: virtualization@lists.linux-foundation.org Cc: linux-ker...@vger.kernel.org Cc: sta...@vger.kernel.org Link: https://lore.kernel.org/r/20200123101000.GB24255@Red Signed-off-by: Gonglei Signed-off-by: Longpeng(Mike) Link: https://lore.kernel.org/r/20200602070501.2023-2-longpe...@huawei.com Signed-off-by: Michael S. Tsirkin Signed-off-by: Sasha Levin --- drivers/crypto/virtio/virtio_crypto_algs.c | 15 ++- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/drivers/crypto/virtio/virtio_crypto_algs.c b/drivers/crypto/virtio/virtio_crypto_algs.c index 772d2b3137c6..fee78ec46bae 100644 --- a/drivers/crypto/virtio/virtio_crypto_algs.c +++ b/drivers/crypto/virtio/virtio_crypto_algs.c @@ -354,13 +354,18 @@ __virtio_crypto_ablkcipher_do_req(struct virtio_crypto_sym_request *vc_sym_req, int err; unsigned long flags; struct scatterlist outhdr, iv_sg, status_sg, **sgs; - int i; u64 dst_len; unsigned int num_out = 0, num_in = 0; int sg_total; uint8_t *iv; + struct scatterlist *sg; src_nents = sg_nents_for_len(req->src, req->nbytes); + if (src_nents < 0) { + pr_err("Invalid number of src SG.\n"); + return src_nents; + } + dst_nents = sg_nents(req->dst); pr_debug("virtio_crypto: Number of sgs (src_nents: %d, dst_nents: %d)\n", @@ -441,12 +446,12 @@ __virtio_crypto_ablkcipher_do_req(struct virtio_crypto_sym_request *vc_sym_req, vc_sym_req->iv = iv; /* Source data */ - for (i = 0; i < src_nents; i++) - sgs[num_out++] = &req->src[i]; + for (sg = req->src; src_nents; sg = sg_next(sg), src_nents--) + sgs[num_out++] = sg; /* Destination data */ - for (i = 0; i < dst_nents; i++) - sgs[num_out + num_in++] = &req->dst[i]; + for (sg = req->dst; sg; sg = sg_next(sg)) + sgs[num_out + num_in++] = sg; /* Status */ sg_init_one(&status_sg, &vc_req->status, sizeof(vc_req->status)); -- 2.25.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 4.14 051/190] crypto: virtio: Fix dest length calculation in __virtio_crypto_skcipher_do_req()
From: Longpeng(Mike) [ Upstream commit d90ca42012db2863a9a30b564a2ace6016594bda ] The src/dst length is not aligned with AES_BLOCK_SIZE(which is 16) in some testcases in tcrypto.ko. For example, the src/dst length of one of cts(cbc(aes))'s testcase is 17, the crypto_virtio driver will set @src_data_len=16 but @dst_data_len=17 in this case and get a wrong at then end. SRC: pp pp pp pp pp pp pp pp pp pp pp pp pp pp pp pp pp (17 bytes) EXP: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc pp (17 bytes) DST: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc 00 (pollute the last bytes) (pp: plaintext cc:ciphertext) Fix this issue by limit the length of dest buffer. Fixes: dbaf0624ffa5 ("crypto: add virtio-crypto driver") Cc: Gonglei Cc: Herbert Xu Cc: "Michael S. Tsirkin" Cc: Jason Wang Cc: "David S. Miller" Cc: virtualization@lists.linux-foundation.org Cc: linux-ker...@vger.kernel.org Cc: sta...@vger.kernel.org Signed-off-by: Longpeng(Mike) Link: https://lore.kernel.org/r/20200602070501.2023-4-longpe...@huawei.com Signed-off-by: Michael S. Tsirkin Signed-off-by: Sasha Levin --- drivers/crypto/virtio/virtio_crypto_algs.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/crypto/virtio/virtio_crypto_algs.c b/drivers/crypto/virtio/virtio_crypto_algs.c index fee78ec46bae..e6b889ce395e 100644 --- a/drivers/crypto/virtio/virtio_crypto_algs.c +++ b/drivers/crypto/virtio/virtio_crypto_algs.c @@ -411,6 +411,7 @@ __virtio_crypto_ablkcipher_do_req(struct virtio_crypto_sym_request *vc_sym_req, goto free; } + dst_len = min_t(unsigned int, req->nbytes, dst_len); pr_debug("virtio_crypto: src_len: %u, dst_len: %llu\n", req->nbytes, dst_len); -- 2.25.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH] drm/virtio: Fix an IS_ERR() vs NULL check in virtio_gpu_object_shmem_init()
The drm_gem_shmem_get_pages_sgt() function returns error pointers on error, it never returns NULL. Fixes: d323bb44e4d2 ("drm/virtio: Call the right shmem helpers") Signed-off-by: Dan Carpenter --- drivers/gpu/drm/virtio/virtgpu_object.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c index 346cef5ce251..0cd5ecf4b3c0 100644 --- a/drivers/gpu/drm/virtio/virtgpu_object.c +++ b/drivers/gpu/drm/virtio/virtgpu_object.c @@ -151,9 +151,9 @@ static int virtio_gpu_object_shmem_init(struct virtio_gpu_device *vgdev, return -EINVAL; shmem->pages = drm_gem_shmem_get_pages_sgt(&bo->base.base); - if (!shmem->pages) { + if (IS_ERR(shmem->pages)) { drm_gem_shmem_unpin(&bo->base.base); - return -EINVAL; + return PTR_ERR(shmem->pages); } if (use_dma_api) { -- 2.27.0 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] drivers\block: Use kobj_to_dev() API
On Fri, Jun 12, 2020 at 03:10:56PM +0800, Wang Qing wrote: > Use kobj_to_dev() API instead of container_of(). > > Signed-off-by: Wang Qing > --- > drivers/block/virtio_blk.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > mode change 100644 => 100755 drivers/block/virtio_blk.c Please fix the '\' -> '/' in the commit message. Looks good otherwise: Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3 1/1] s390: virtio: let arch accept devices without IOMMU feature
On Fri, 19 Jun 2020 11:20:51 +0200 Cornelia Huck wrote: > > > + if (arch_needs_virtio_iommu_platform(dev) && > > > + !virtio_has_feature(dev, VIRTIO_F_IOMMU_PLATFORM)) { > > > + dev_warn(&dev->dev, > > > + "virtio: device must provide > > > VIRTIO_F_IOMMU_PLATFORM\n"); > > > > I'm not sure, divulging the current Linux name of this feature bit is a > > good idea, but if everybody else is fine with this, I don't care that > > Not sure if that feature name will ever change, as it is exported in > headers. At most, we might want to add the new ACCESS_PLATFORM define > and keep the old one, but that would still mean some churn. > > > much. An alternative would be: > > "virtio: device falsely claims to have full access to the memory, > > aborting the device" > > "virtio: device does not work with limited memory access" ? > > But no issue with keeping the current message. I think I prefer Conny's version, but no strong feelings here. Halil ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] virtio-mem: Fix build error due to improper use 'select'
On 19.06.20 10:03, Weilong Chen wrote: > As noted in: > https://www.kernel.org/doc/Documentation/kbuild/kconfig-language.txt > "select should be used with care. select will force a symbol to a > value without visiting the dependencies." Right, rings a bell. > Config VIRTIO_MEM should not select CONTIG_ALLOC directly. > Otherwise it will cause an error: > https://bugzilla.kernel.org/show_bug.cgi?id=208245 Thanks! Acked-by: David Hildenbrand > > Signed-off-by: Weilong Chen > --- > drivers/virtio/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig > index 5809e5f5b157..5c92e4a50882 100644 > --- a/drivers/virtio/Kconfig > +++ b/drivers/virtio/Kconfig > @@ -85,7 +85,7 @@ config VIRTIO_MEM > depends on VIRTIO > depends on MEMORY_HOTPLUG_SPARSE > depends on MEMORY_HOTREMOVE > - select CONTIG_ALLOC > + depends on CONTIG_ALLOC > help >This driver provides access to virtio-mem paravirtualized memory >devices, allowing to hotplug and hotunplug memory. > -- Thanks, David / dhildenb ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] virtio-mem: Fix build error due to improper use 'select'
On Fri, Jun 19, 2020 at 04:03:33PM +0800, Weilong Chen wrote: > As noted in: > https://www.kernel.org/doc/Documentation/kbuild/kconfig-language.txt > "select should be used with care. select will force a symbol to a > value without visiting the dependencies." > Config VIRTIO_MEM should not select CONTIG_ALLOC directly. > Otherwise it will cause an error: > https://bugzilla.kernel.org/show_bug.cgi?id=208245 > > Signed-off-by: Weilong Chen Cc virtio mem maintainer: M: David Hildenbrand > --- > drivers/virtio/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig > index 5809e5f5b157..5c92e4a50882 100644 > --- a/drivers/virtio/Kconfig > +++ b/drivers/virtio/Kconfig > @@ -85,7 +85,7 @@ config VIRTIO_MEM > depends on VIRTIO > depends on MEMORY_HOTPLUG_SPARSE > depends on MEMORY_HOTREMOVE > - select CONTIG_ALLOC > + depends on CONTIG_ALLOC > help >This driver provides access to virtio-mem paravirtualized memory >devices, allowing to hotplug and hotunplug memory. > -- > 2.17.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3 1/1] s390: virtio: let arch accept devices without IOMMU feature
On Thu, 18 Jun 2020 00:29:56 +0200 Halil Pasic wrote: > On Wed, 17 Jun 2020 12:43:57 +0200 > Pierre Morel wrote: > > > An architecture protecting the guest memory against unauthorized host > > access may want to enforce VIRTIO I/O device protection through the > > use of VIRTIO_F_IOMMU_PLATFORM. > > > > Let's give a chance to the architecture to accept or not devices > > without VIRTIO_F_IOMMU_PLATFORM. > > > [..] > > > I'm still not really satisfied with your commit message, furthermore > I did some thinking about the abstraction you introduce here. I will > give a short analysis of that, but first things first. Your patch does > the job of preventing calamity, and the details can be changed any time, > thus: > > Acked-by: Halil Pasic > > Regarding the interaction of architecture specific code with virtio core, > I believe we could have made the interface more generic. > > One option is to introduce virtio_arch_finalize_features(), a hook that > could reject any feature that is inappropriate. s/any feature/any combination of features/ This sounds like a good idea (for a later update). > > Another option would be to find a common name for is_prot_virt_guest() > (arch/s390) sev_active() (arch/x86) and is_secure_guest() (arch/powerpc) > and use that instead of arch_needs_virtio_iommu_platform() and where-ever > appropriate. Currently we seem to want this info in driver code only for > virtio, but if the virtio driver has a legitimate need to know, other > drivers may as well have a legitimate need to know. For example if we > wanted to protect ourselves in ccw device drivers from somebody > setting up a vfio-ccw device and attach it to the prot-virt guest (AFAICT > we only lack guest enablement for this) such a function could be useful. I'm not really sure if we can find enough commonality between architectures, unless you propose to have a function for checking things like device memory only. > > But since this can be rewritten any time, let's go with the option > people already agree with, instead of more discussion. Yes, there's nothing wrong with the patch as-is. Acked-by: Cornelia Huck Which tree should this go through? Virtio? s390? > > Just another question. Do we want this backported? Do we need cc stable? It does change behaviour of virtio-ccw devices; but then, it only fences off configurations that would not have worked anyway. Distributions should probably pick this; but I do not consider it strictly a "fix" (more a mitigation for broken configurations), so I'm not sure whether stable applies. > [..] > > > > int virtio_finalize_features(struct virtio_device *dev) > > { > > int ret = dev->config->finalize_features(dev); > > @@ -179,6 +194,13 @@ int virtio_finalize_features(struct virtio_device *dev) > > if (!virtio_has_feature(dev, VIRTIO_F_VERSION_1)) > > return 0; > > > > + if (arch_needs_virtio_iommu_platform(dev) && > > + !virtio_has_feature(dev, VIRTIO_F_IOMMU_PLATFORM)) { > > + dev_warn(&dev->dev, > > +"virtio: device must provide > > VIRTIO_F_IOMMU_PLATFORM\n"); > > I'm not sure, divulging the current Linux name of this feature bit is a > good idea, but if everybody else is fine with this, I don't care that Not sure if that feature name will ever change, as it is exported in headers. At most, we might want to add the new ACCESS_PLATFORM define and keep the old one, but that would still mean some churn. > much. An alternative would be: > "virtio: device falsely claims to have full access to the memory, > aborting the device" "virtio: device does not work with limited memory access" ? But no issue with keeping the current message. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] virtio-mem: Fix build error due to improper use 'select'
On 2020/6/19 下午4:03, Weilong Chen wrote: As noted in: https://www.kernel.org/doc/Documentation/kbuild/kconfig-language.txt "select should be used with care. select will force a symbol to a value without visiting the dependencies." Config VIRTIO_MEM should not select CONTIG_ALLOC directly. Otherwise it will cause an error: https://bugzilla.kernel.org/show_bug.cgi?id=208245 Signed-off-by: Weilong Chen --- drivers/virtio/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig index 5809e5f5b157..5c92e4a50882 100644 --- a/drivers/virtio/Kconfig +++ b/drivers/virtio/Kconfig @@ -85,7 +85,7 @@ config VIRTIO_MEM depends on VIRTIO depends on MEMORY_HOTPLUG_SPARSE depends on MEMORY_HOTREMOVE - select CONTIG_ALLOC + depends on CONTIG_ALLOC help This driver provides access to virtio-mem paravirtualized memory devices, allowing to hotplug and hotunplug memory. Acked-by: Jason Wang ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization