Re: [Xen-devel] [PATCH 11/29] drivers, media: convert cx88_core.refcount from atomic_t to refcount_t

[Sergei Shtylyov]

On 3/7/2017 10:52 AM, Reshetova, Elena wrote:


refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.

Signed-off-by: Elena Reshetova 
Signed-off-by: Hans Liljestrand 
Signed-off-by: Kees Cook 
Signed-off-by: David Windsor 

[...]

diff --git a/drivers/media/pci/cx88/cx88.h b/drivers/media/pci/cx88/cx88.h
index 115414c..16c1313 100644
--- a/drivers/media/pci/cx88/cx88.h
+++ b/drivers/media/pci/cx88/cx88.h

[...]

@@ -339,7 +340,7 @@ struct cx8802_dev;

 struct cx88_core {
struct list_head   devlist;
-   atomic_t   refcount;
+   refcount_t   refcount;


Could you please keep the name aligned with above and below?


You mean "not aligned" to devlist, but with a shift like it was before?


   I mean aligned, like it was before. :-)


Sure, will fix. Is the patch ok otherwise?


   I haven't noticed anything else...


Best Regards,
Elena.

[...]

MBR, Sergei

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 11/29] drivers, media: convert cx88_core.refcount from atomic_t to refcount_t

[Sergei Shtylyov]

Hello.

On 03/06/2017 05:20 PM, Elena Reshetova wrote:


refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.

Signed-off-by: Elena Reshetova 
Signed-off-by: Hans Liljestrand 
Signed-off-by: Kees Cook 
Signed-off-by: David Windsor 

[...]

diff --git a/drivers/media/pci/cx88/cx88.h b/drivers/media/pci/cx88/cx88.h
index 115414c..16c1313 100644
--- a/drivers/media/pci/cx88/cx88.h
+++ b/drivers/media/pci/cx88/cx88.h
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 

 #include 
 #include 
@@ -339,7 +340,7 @@ struct cx8802_dev;

 struct cx88_core {
struct list_head   devlist;
-   atomic_t   refcount;
+   refcount_t   refcount;


   Could you please keep the name aligned with above and below?



/* board name */
intnr;



MBR, Sergei


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] xen-netfront: set real_num_tx_queues to zreo avoid to trigger BUG_ON

[Sergei Shtylyov]

On 02/20/2016 04:27 AM, Gonglei wrote:


It's possible for a race condition to exist between xennet_open() and
talk_to_netback(). After invoking netfront_probe() then other
threads or processes invoke xennet_open (such as NetworkManager)
immediately may trigger BUG_ON(). Besides, we also should reset
real_num_tx_queues in xennet_destroy_queues().

 [ 3324.658057] kernel BUG at include/linux/netdevice.h:508!
 [ 3324.658057] invalid opcode:  [#1] SMP
 [ 3324.658057] CPU: 0 PID: 662 Comm: NetworkManager Tainted: G
 [] ? raw_notifier_call_chain+0x16/0x20
 [] __dev_open+0xce/0x150
 [] __dev_change_flags+0xa1/0x170
 [] dev_change_flags+0x29/0x70
 [] do_setlink+0x39f/0xb40
 [] ? nla_parse+0x32/0x120
 [] rtnl_newlink+0x604/0x900
 [] ? netlink_unicast+0x193/0x1c0
 [] ? security_capable+0x18/0x20
 [] ? ns_capable+0x2d/0x60
 [] rtnetlink_rcv_msg+0xf5/0x270
 [] ? rhashtable_lookup_compare+0x5d/0xa0
 [] ? rtnetlink_rcv+0x40/0x40
 [] netlink_rcv_skb+0xb9/0xe0
 [] rtnetlink_rcv+0x2c/0x40
 [] netlink_unicast+0x12d/0x1c0
 [] netlink_sendmsg+0x4d3/0x630
 [] ? sock_has_perm+0x72/0x90
 [] do_sock_sendmsg+0x9f/0xc0
 [ 3324.703482] RIP  [] xennet_open+0x180/0x182 
[xen_netfront]

CC: David S. Miller 
Signed-off-by: Gonglei 


   Full name required for this tag.

[...]

MBR, Sergei


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 32/32] virtio_ring: use virt_store_mb

[Sergei Shtylyov]

Hello.

On 12/31/2015 10:09 PM, Michael S. Tsirkin wrote:


We need a full barrier after writing out event index, using
virt_store_mb there seems better than open-coding.  As usual, we need a
wrapper to account for strong barriers.

It's tempting to use this in vhost as well, for that, we'll
need a variant of smp_store_mb that works on __user pointers.

Signed-off-by: Michael S. Tsirkin 
---
  include/linux/virtio_ring.h  | 12 
  drivers/virtio/virtio_ring.c | 15 +--
  2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
index f3fa55b..3a74d91 100644
--- a/include/linux/virtio_ring.h
+++ b/include/linux/virtio_ring.h
@@ -45,6 +45,18 @@ static inline void virtio_wmb(bool weak_barriers)
wmb();
  }

+static inline void virtio_store_mb(bool weak_barriers,
+  __virtio16 *p, __virtio16 v)
+{
+   if (weak_barriers)
+   virt_store_mb(*p, v);
+   else
+   {


   The kernel coding style dictates:

if (weak_barriers) {
virt_store_mb(*p, v);
} else {


+   WRITE_ONCE(*p, v);
+   mb();
+   }
+}
+

[...]

MBR, Sergei


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 3/3] net/xen-netback: Don't mix hexa and decimal with 0x in the printf format

[Sergei Shtylyov]

Hello.

On 06/16/2015 10:10 PM, Julien Grall wrote:


Append 0x to all %x in order to avoid while reading when there is other
decimal value in the log.



Also replace some of the hexadecimal print to decimal to uniformize the
format with netfront.



Signed-off-by: Julien Grall julien.gr...@citrix.com
Cc: Wei Liu wei.l...@citrix.com
Cc: Ian Campbell ian.campb...@citrix.com
Cc: net...@vger.kernel.org



---
 Changes in v4:
 - Patch added
---
  drivers/net/xen-netback/netback.c | 14 +++---
  1 file changed, 7 insertions(+), 7 deletions(-)



diff --git a/drivers/net/xen-netback/netback.c 
b/drivers/net/xen-netback/netback.c
index ba3ae30..11bd9d8 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c

[...]

@@ -874,7 +874,7 @@ static inline void xenvif_grant_handle_set(struct 
xenvif_queue *queue,
if (unlikely(queue-grant_tx_handle[pending_idx] !=
 NETBACK_INVALID_HANDLE)) {
netdev_err(queue-vif-dev,
-  Trying to overwrite active handle! pending_idx: 
%x\n,
+  Trying to overwrite active handle! pending_idx: 
0x%x\n,


   Using %#x is shorter ind does the same.


   pending_idx);
BUG();
}
@@ -887,7 +887,7 @@ static inline void xenvif_grant_handle_reset(struct 
xenvif_queue *queue,
if (unlikely(queue-grant_tx_handle[pending_idx] ==
 NETBACK_INVALID_HANDLE)) {
netdev_err(queue-vif-dev,
-  Trying to unmap invalid handle! pending_idx: %x\n,
+  Trying to unmap invalid handle! pending_idx: 
0x%x\n,


   Same here.

[...]

@@ -1731,7 +1731,7 @@ void xenvif_idx_unmap(struct xenvif_queue *queue, u16 
pending_idx)
queue-mmap_pages[pending_idx], 1);
if (ret) {
netdev_err(queue-vif-dev,
-  Unmap fail: ret: %d pending_idx: %d host_addr: %llx 
handle: %x status: %d\n,
+  Unmap fail: ret: %d pending_idx: %d host_addr: %llx 
handle: 0x%x status: %d\n,


   And here.

[...]

WBR, Sergei


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 3/3] net/xen-netback: Don't mix hexa and decimal with 0x in the printf format

[Sergei Shtylyov]

Hello.

On 06/17/2015 01:09 AM, Joe Perches wrote:


Append 0x to all %x in order to avoid while reading when there is other
decimal value in the log.



[]



@@ -874,7 +874,7 @@ static inline void xenvif_grant_handle_set(struct 
xenvif_queue *queue,
if (unlikely(queue-grant_tx_handle[pending_idx] !=
 NETBACK_INVALID_HANDLE)) {
netdev_err(queue-vif-dev,
-  Trying to overwrite active handle! pending_idx: 
%x\n,
+  Trying to overwrite active handle! pending_idx: 
0x%x\n,



 Using %#x is shorter ind does the same.



That's true, but it's also far less common.


   Which is a pity... People just don't know the format specifiers well 
enough. :-(



$ git grep -E %#[\*\d\.]*x | wc -l
1419
$ git grep 0x% | wc -l
29844


   Which means 29 KB could theoretically be saved on allyesconfig build. :-)
(Actually less since the width specifiers will likely need to be fixed where 
present.)


WBR, Sergei


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] xen-netfront: transmit fully GSO-sized packets

[Sergei Shtylyov]

Hello.

On 3/26/2015 2:13 PM, Jonathan Davies wrote:


xen-netfront limits transmitted skbs to be at most 44 segments in size. However,
GSO permits up to 65536 bytes, which means a maximum of 45 segments of 1448
bytes each. This slight reduction in the size of packets means a slight loss in
efficiency.



Since c/s 9ecd1a75d, xen-netfront sets gso_max_size to


   c/s == commit?
   Please also specify that commit's summary line in parens.


 XEN_NETIF_MAX_TX_SIZE - MAX_TCP_HEADER,
where XEN_NETIF_MAX_TX_SIZE is 65535 bytes.



The calculation used by tcp_tso_autosize (and also tcp_xmit_size_goal since c/s
6c09fa09d) in determining when to split an skb into two is


   Likewise.


 sk-sk_gso_max_size - 1 - MAX_TCP_HEADER.



So the maximum permitted size of an skb is calculated to be
 (XEN_NETIF_MAX_TX_SIZE - MAX_TCP_HEADER) - 1 - MAX_TCP_HEADER.



Intuitively, this looks like the wrong formula -- we don't need two TCP headers.
Instead, there is no need to deviate from the default gso_max_size of 65536 as
this already accommodates the size of the header.



Currently, the largest skb transmitted by netfront is 63712 bytes (44 segments
of 1448 bytes each), as observed via tcpdump. This patch makes netfront send
skbs of up to 65160 bytes (45 segments of 1448 bytes each).



Fixes: 9ecd1a75d977 (xen-netfront: reduce gso_max_size to account for max TCP 
header)


   Ah, here's the summary for the first mentioned commit...


Signed-off-by: Jonathan Davies jonathan.dav...@citrix.com


WBR, Sergei


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCHv1 1/2] xen-netback: return correct ethtool stats

[Sergei Shtylyov]

Hello.

On 3/3/2015 7:26 PM, David Vrabel wrote:


Use correct pointer arithmetic to get the pointer to each stat.



Signed-off-by: David Vrabel david.vra...@citrix.com
---
  drivers/net/xen-netback/interface.c |3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)



diff --git a/drivers/net/xen-netback/interface.c 
b/drivers/net/xen-netback/interface.c
index f38227a..3aa8648 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -340,12 +340,11 @@ static void xenvif_get_ethtool_stats(struct net_device 
*dev,
unsigned int num_queues = vif-num_queues;
int i;
unsigned int queue_index;
-   struct xenvif_stats *vif_stats;

for (i = 0; i  ARRAY_SIZE(xenvif_stats); i++) {
unsigned long accum = 0;
for (queue_index = 0; queue_index  num_queues; ++queue_index) {
-   vif_stats = vif-queues[queue_index].stats;
+   void *vif_stats = vif-queues[queue_index].stats;


   Need empty line after declaration; checkpatch.pl should have complained here.


accum += *(unsigned long *)(vif_stats + 
xenvif_stats[i].offset);
}
data[i] = accum;


WBR, Sergei


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel