* [PATCH 0/2] drivers: Move visorbus from staging to drivers/visorbus
@ 2017-11-17 17:27 David Kershner
2017-11-17 17:27 ` [PATCH 1/2] staging: unisys: visorbus: address theoretical int overflows David Kershner
2017-11-17 17:27 ` [PATCH 2/2] drivers: visorbus: move driver out of staging David Kershner
0 siblings, 2 replies; 8+ messages in thread
From: David Kershner @ 2017-11-17 17:27 UTC (permalink / raw)
To: david.kershner, gregkh, jes.sorensen, linux-kernel,
driverdev-devel, sparmaintainer, erik.arfvidson, wadgaonkarsam
This patch series fixes an issue that was reported by Dan Carpenter, moves
the necessary include files to include/linux/visorbus and then moves the
visorbus driver from drivers/staging/unisys to drivers/visorbus.
Greg, thanks for the review of visorbus and for the assistance to get the
driver to this state.
David Kershner (1):
drivers: visorbus: move driver out of staging
Tim Sell (1):
staging: unisys: visorbus: address theoretical int overflows
MAINTAINERS | 2 ++
drivers/Kconfig | 2 ++
drivers/Makefile | 1 +
drivers/staging/unisys/Kconfig | 1 -
drivers/staging/unisys/Makefile | 1 -
drivers/staging/unisys/include/iochannel.h | 3 +--
drivers/staging/unisys/visorhba/visorhba_main.c | 2 +-
drivers/staging/unisys/visorinput/visorinput.c | 2 +-
drivers/staging/unisys/visornic/visornic_main.c | 2 +-
drivers/{staging/unisys => }/visorbus/Kconfig | 4 +++-
drivers/{staging/unisys => }/visorbus/Makefile | 2 --
drivers/{staging/unisys => }/visorbus/controlvmchannel.h | 3 +--
drivers/{staging/unisys => }/visorbus/vbuschannel.h | 2 +-
drivers/{staging/unisys => }/visorbus/visorbus_main.c | 2 +-
drivers/{staging/unisys => }/visorbus/visorbus_private.h | 2 +-
drivers/{staging/unisys => }/visorbus/visorchannel.c | 2 +-
drivers/{staging/unisys => }/visorbus/visorchipset.c | 9 +++++----
.../staging/unisys/include => include/linux/visorbus}/visorbus.h | 0
.../unisys/include => include/linux/visorbus}/visorchannel.h | 0
19 files changed, 22 insertions(+), 20 deletions(-)
rename drivers/{staging/unisys => }/visorbus/Kconfig (90%)
rename drivers/{staging/unisys => }/visorbus/Makefile (81%)
rename drivers/{staging/unisys => }/visorbus/controlvmchannel.h (99%)
rename drivers/{staging/unisys => }/visorbus/vbuschannel.h (98%)
rename drivers/{staging/unisys => }/visorbus/visorbus_main.c (99%)
rename drivers/{staging/unisys => }/visorbus/visorbus_private.h (98%)
rename drivers/{staging/unisys => }/visorbus/visorchannel.c (99%)
rename drivers/{staging/unisys => }/visorbus/visorchipset.c (99%)
rename {drivers/staging/unisys/include => include/linux/visorbus}/visorbus.h (100%)
rename {drivers/staging/unisys/include => include/linux/visorbus}/visorchannel.h (100%)
--
1.9.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] staging: unisys: visorbus: address theoretical int overflows
2017-11-17 17:27 [PATCH 0/2] drivers: Move visorbus from staging to drivers/visorbus David Kershner
@ 2017-11-17 17:27 ` David Kershner
2017-11-28 14:11 ` Dan Carpenter
2017-11-17 17:27 ` [PATCH 2/2] drivers: visorbus: move driver out of staging David Kershner
1 sibling, 1 reply; 8+ messages in thread
From: David Kershner @ 2017-11-17 17:27 UTC (permalink / raw)
To: david.kershner, gregkh, jes.sorensen, linux-kernel,
driverdev-devel, sparmaintainer, erik.arfvidson, wadgaonkarsam
Cc: Tim Sell
From: Tim Sell <Timothy.Sell@unisys.com>
Add necessary casting to several places where we were doing 32-bit
arithmetic (unsigned) to produce a 64-bit (unsigned long) result, to
prevent the theoretical possibility of a 32-bit overflow during the
arithmetic.
FYI, these are unsigned long:
ctx->param_bytes
ctx->allocbytes
These are unsigned int:
bytes
phdr->name_offset
phdr->name_length
Here is the test program demonstrating why we really need the casts:
void main()
{
unsigned int i;
unsigned long il;
printf("sizeof(int) =%dn",sizeof(i));
printf("sizeof(long)=%dn",sizeof(il));
i = (unsigned int)((((unsigned long)(1)) << 32) - 1);
printf("i = %un", i);
il = i+1;
printf("adding 1 withOUT cast = %lun", il);
il = (unsigned long)i+1;
printf("adding 1 WITH cast = %lun", il);
}
[selltc@mac tmp]$ gcc x.c -o x.out
[selltc@mac tmp]$ ./x.out
sizeof(int) =4
sizeof(long)=8
i = 4294967295
adding 1 withOUT cast = 0
adding 1 WITH cast = 4294967296
Signed-off-by: Tim Sell <Timothy.Sell@unisys.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: David Kershner <david.kershner@unisys.com>
Reviewed-by: David Binder <david.binder@unisys.com>
---
drivers/staging/unisys/visorbus/visorchipset.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/unisys/visorbus/visorchipset.c b/drivers/staging/unisys/visorbus/visorchipset.c
index fed554a4..ef2823a 100644
--- a/drivers/staging/unisys/visorbus/visorchipset.c
+++ b/drivers/staging/unisys/visorbus/visorchipset.c
@@ -590,7 +590,8 @@ static void *parser_name_get(struct parser_context *ctx)
struct visor_controlvm_parameters_header *phdr;
phdr = &ctx->data;
- if (phdr->name_offset + phdr->name_length > ctx->param_bytes)
+ if ((unsigned long)phdr->name_offset +
+ (unsigned long)phdr->name_length > ctx->param_bytes)
return NULL;
ctx->curr = (char *)&phdr + phdr->name_offset;
ctx->bytes_remaining = phdr->name_length;
@@ -1317,13 +1318,13 @@ static void parser_done(struct parser_context *ctx)
static struct parser_context *parser_init_stream(u64 addr, u32 bytes,
bool *retry)
{
- int allocbytes;
+ unsigned long allocbytes;
struct parser_context *ctx;
void *mapping;
*retry = false;
/* alloc an extra byte to ensure payload is \0 terminated */
- allocbytes = bytes + 1 + (sizeof(struct parser_context) -
+ allocbytes = (unsigned long)bytes + 1 + (sizeof(struct parser_context) -
sizeof(struct visor_controlvm_parameters_header));
if ((chipset_dev->controlvm_payload_bytes_buffered + bytes) >
MAX_CONTROLVM_PAYLOAD_BYTES) {
--
1.9.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] drivers: visorbus: move driver out of staging
2017-11-17 17:27 [PATCH 0/2] drivers: Move visorbus from staging to drivers/visorbus David Kershner
2017-11-17 17:27 ` [PATCH 1/2] staging: unisys: visorbus: address theoretical int overflows David Kershner
@ 2017-11-17 17:27 ` David Kershner
2017-11-17 19:17 ` Christoph Hellwig
2017-11-18 10:26 ` Greg KH
1 sibling, 2 replies; 8+ messages in thread
From: David Kershner @ 2017-11-17 17:27 UTC (permalink / raw)
To: david.kershner, gregkh, jes.sorensen, linux-kernel,
driverdev-devel, sparmaintainer, erik.arfvidson, wadgaonkarsam
The s-Par header files that are referenced by all s-Par drivers, are being
moved into include/linux/visorbus. Move the visorbus driver out of staging
and modify the configuration and makefiles so they now reference the new
file, this required moving some lines from drivers/staging/unisys/Kconfig
over to drives/visorbus/Kconfig to make sure dependencies were met.
Visorbus will now just live in the /drivers directory.
Signed-off-by: David Kershner <david.kershner@unisys.com>
Reviewed-by: David Binder <david.binder@unisys.com>
Reviewed-by: Tim Sell <timothy.sell@unisys.com>
---
MAINTAINERS | 2 ++
drivers/Kconfig | 2 ++
drivers/Makefile | 1 +
drivers/staging/unisys/Kconfig | 1 -
drivers/staging/unisys/Makefile | 1 -
drivers/staging/unisys/include/iochannel.h | 3 +--
drivers/staging/unisys/visorhba/visorhba_main.c | 2 +-
drivers/staging/unisys/visorinput/visorinput.c | 2 +-
drivers/staging/unisys/visornic/visornic_main.c | 2 +-
drivers/{staging/unisys => }/visorbus/Kconfig | 4 +++-
drivers/{staging/unisys => }/visorbus/Makefile | 2 --
drivers/{staging/unisys => }/visorbus/controlvmchannel.h | 3 +--
drivers/{staging/unisys => }/visorbus/vbuschannel.h | 2 +-
drivers/{staging/unisys => }/visorbus/visorbus_main.c | 2 +-
drivers/{staging/unisys => }/visorbus/visorbus_private.h | 2 +-
drivers/{staging/unisys => }/visorbus/visorchannel.c | 2 +-
drivers/{staging/unisys => }/visorbus/visorchipset.c | 2 +-
{drivers/staging/unisys/include => include/linux/visorbus}/visorbus.h | 0
.../staging/unisys/include => include/linux/visorbus}/visorchannel.h | 0
19 files changed, 18 insertions(+), 17 deletions(-)
rename drivers/{staging/unisys => }/visorbus/Kconfig (90%)
rename drivers/{staging/unisys => }/visorbus/Makefile (81%)
rename drivers/{staging/unisys => }/visorbus/controlvmchannel.h (99%)
rename drivers/{staging/unisys => }/visorbus/vbuschannel.h (98%)
rename drivers/{staging/unisys => }/visorbus/visorbus_main.c (99%)
rename drivers/{staging/unisys => }/visorbus/visorbus_private.h (98%)
rename drivers/{staging/unisys => }/visorbus/visorchannel.c (99%)
rename drivers/{staging/unisys => }/visorbus/visorchipset.c (99%)
rename {drivers/staging/unisys/include => include/linux/visorbus}/visorbus.h (100%)
rename {drivers/staging/unisys/include => include/linux/visorbus}/visorchannel.h (100%)
diff --git a/MAINTAINERS b/MAINTAINERS
index 650aa0e..a1df0e9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13835,6 +13835,8 @@ UNISYS S-PAR DRIVERS
M: David Kershner <david.kershner@unisys.com>
L: sparmaintainer@unisys.com (Unisys internal)
S: Supported
+F: include/linux/visorbus/
+F: drivers/visorbus/
F: drivers/staging/unisys/
UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER
diff --git a/drivers/Kconfig b/drivers/Kconfig
index 152744c..ef5fb83 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -211,4 +211,6 @@ source "drivers/mux/Kconfig"
source "drivers/opp/Kconfig"
+source "drivers/visorbus/Kconfig"
+
endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index 1d034b6..d5061ed 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -183,3 +183,4 @@ obj-$(CONFIG_FPGA) += fpga/
obj-$(CONFIG_FSI) += fsi/
obj-$(CONFIG_TEE) += tee/
obj-$(CONFIG_MULTIPLEXER) += mux/
+obj-$(CONFIG_UNISYS_VISORBUS) += visorbus/
diff --git a/drivers/staging/unisys/Kconfig b/drivers/staging/unisys/Kconfig
index 4f1f5e6..688b5e3 100644
--- a/drivers/staging/unisys/Kconfig
+++ b/drivers/staging/unisys/Kconfig
@@ -11,7 +11,6 @@ menuconfig UNISYSSPAR
if UNISYSSPAR
-source "drivers/staging/unisys/visorbus/Kconfig"
source "drivers/staging/unisys/visornic/Kconfig"
source "drivers/staging/unisys/visorinput/Kconfig"
source "drivers/staging/unisys/visorhba/Kconfig"
diff --git a/drivers/staging/unisys/Makefile b/drivers/staging/unisys/Makefile
index 20eb098..e45f44b 100644
--- a/drivers/staging/unisys/Makefile
+++ b/drivers/staging/unisys/Makefile
@@ -1,7 +1,6 @@
#
# Makefile for Unisys SPAR drivers
#
-obj-$(CONFIG_UNISYS_VISORBUS) += visorbus/
obj-$(CONFIG_UNISYS_VISORNIC) += visornic/
obj-$(CONFIG_UNISYS_VISORINPUT) += visorinput/
obj-$(CONFIG_UNISYS_VISORHBA) += visorhba/
diff --git a/drivers/staging/unisys/include/iochannel.h b/drivers/staging/unisys/include/iochannel.h
index 5cd407c..6e401a7 100644
--- a/drivers/staging/unisys/include/iochannel.h
+++ b/drivers/staging/unisys/include/iochannel.h
@@ -43,8 +43,7 @@
#include <linux/uuid.h>
#include <linux/skbuff.h>
-
-#include "visorchannel.h"
+#include <linux/visorbus/visorchannel.h>
/*
* Must increment these whenever you insert or delete fields within this channel
diff --git a/drivers/staging/unisys/visorhba/visorhba_main.c b/drivers/staging/unisys/visorhba/visorhba_main.c
index 0bcd3ac..88b04c4 100644
--- a/drivers/staging/unisys/visorhba/visorhba_main.c
+++ b/drivers/staging/unisys/visorhba/visorhba_main.c
@@ -19,12 +19,12 @@
#include <linux/idr.h>
#include <linux/module.h>
#include <linux/seq_file.h>
+#include <linux/visorbus/visorbus.h>
#include <scsi/scsi.h>
#include <scsi/scsi_host.h>
#include <scsi/scsi_cmnd.h>
#include <scsi/scsi_device.h>
-#include "visorbus.h"
#include "iochannel.h"
/* The Send and Receive Buffers of the IO Queue may both be full */
diff --git a/drivers/staging/unisys/visorinput/visorinput.c b/drivers/staging/unisys/visorinput/visorinput.c
index 450f003..4d88e6c 100644
--- a/drivers/staging/unisys/visorinput/visorinput.c
+++ b/drivers/staging/unisys/visorinput/visorinput.c
@@ -25,8 +25,8 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/uuid.h>
+#include <linux/visorbus/visorbus.h>
-#include "visorbus.h"
#include "ultrainputreport.h"
/* Keyboard channel {c73416d0-b0b8-44af-b304-9d2ae99f1b3d} */
diff --git a/drivers/staging/unisys/visornic/visornic_main.c b/drivers/staging/unisys/visornic/visornic_main.c
index 735d7e5..9fd2c8c 100644
--- a/drivers/staging/unisys/visornic/visornic_main.c
+++ b/drivers/staging/unisys/visornic/visornic_main.c
@@ -25,8 +25,8 @@
#include <linux/kthread.h>
#include <linux/skbuff.h>
#include <linux/rtnetlink.h>
+#include <linux/visorbus/visorbus.h>
-#include "visorbus.h"
#include "iochannel.h"
#define VISORNIC_INFINITE_RSP_WAIT 0
diff --git a/drivers/staging/unisys/visorbus/Kconfig b/drivers/visorbus/Kconfig
similarity index 90%
rename from drivers/staging/unisys/visorbus/Kconfig
rename to drivers/visorbus/Kconfig
index 5113880..a99285a 100644
--- a/drivers/staging/unisys/visorbus/Kconfig
+++ b/drivers/visorbus/Kconfig
@@ -4,7 +4,9 @@
config UNISYS_VISORBUS
tristate "Unisys visorbus driver"
- depends on UNISYSSPAR
+ depends on X86_64 && !UML
+ select PCI
+ select ACPI
---help---
The visorbus driver is a virtualized bus for the Unisys s-Par firmware.
Virtualized devices allow Linux guests on a system to share disks and
diff --git a/drivers/staging/unisys/visorbus/Makefile b/drivers/visorbus/Makefile
similarity index 81%
rename from drivers/staging/unisys/visorbus/Makefile
rename to drivers/visorbus/Makefile
index 784cdc1..e8df59d 100644
--- a/drivers/staging/unisys/visorbus/Makefile
+++ b/drivers/visorbus/Makefile
@@ -8,5 +8,3 @@ obj-$(CONFIG_UNISYS_VISORBUS) += visorbus.o
visorbus-y := visorbus_main.o
visorbus-y += visorchannel.o
visorbus-y += visorchipset.o
-
-ccflags-y += -Idrivers/staging/unisys/include
diff --git a/drivers/staging/unisys/visorbus/controlvmchannel.h b/drivers/visorbus/controlvmchannel.h
similarity index 99%
rename from drivers/staging/unisys/visorbus/controlvmchannel.h
rename to drivers/visorbus/controlvmchannel.h
index 9ee9886..3e173ea 100644
--- a/drivers/staging/unisys/visorbus/controlvmchannel.h
+++ b/drivers/visorbus/controlvmchannel.h
@@ -17,8 +17,7 @@
#define __CONTROLVMCHANNEL_H__
#include <linux/uuid.h>
-
-#include "visorchannel.h"
+#include <linux/visorbus/visorchannel.h>
/* {2B3C2D10-7EF5-4ad8-B966-3448B7386B3D} */
#define VISOR_CONTROLVM_CHANNEL_GUID \
diff --git a/drivers/staging/unisys/visorbus/vbuschannel.h b/drivers/visorbus/vbuschannel.h
similarity index 98%
rename from drivers/staging/unisys/visorbus/vbuschannel.h
rename to drivers/visorbus/vbuschannel.h
index 981b180..261fe79 100644
--- a/drivers/staging/unisys/visorbus/vbuschannel.h
+++ b/drivers/visorbus/vbuschannel.h
@@ -26,7 +26,7 @@
*/
#include <linux/uuid.h>
-#include "visorchannel.h"
+#include <linux/visorbus/visorchannel.h>
/* {193b331b-c58f-11da-95a9-00e08161165f} */
#define VISOR_VBUS_CHANNEL_GUID \
diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c b/drivers/visorbus/visorbus_main.c
similarity index 99%
rename from drivers/staging/unisys/visorbus/visorbus_main.c
rename to drivers/visorbus/visorbus_main.c
index b604d0c..706eb6b 100644
--- a/drivers/staging/unisys/visorbus/visorbus_main.c
+++ b/drivers/visorbus/visorbus_main.c
@@ -17,9 +17,9 @@
#include <linux/debugfs.h>
#include <linux/module.h>
#include <linux/slab.h>
+#include <linux/visorbus/visorbus.h>
#include <linux/uuid.h>
-#include "visorbus.h"
#include "visorbus_private.h"
static const guid_t visor_vbus_channel_guid = VISOR_VBUS_CHANNEL_GUID;
diff --git a/drivers/staging/unisys/visorbus/visorbus_private.h b/drivers/visorbus/visorbus_private.h
similarity index 98%
rename from drivers/staging/unisys/visorbus/visorbus_private.h
rename to drivers/visorbus/visorbus_private.h
index 4a8b12d..8183e68 100644
--- a/drivers/staging/unisys/visorbus/visorbus_private.h
+++ b/drivers/visorbus/visorbus_private.h
@@ -18,10 +18,10 @@
#include <linux/uuid.h>
#include <linux/utsname.h>
+#include <linux/visorbus/visorbus.h>
#include "controlvmchannel.h"
#include "vbuschannel.h"
-#include "visorbus.h"
struct visor_device *visorbus_get_device_by_id(u32 bus_no, u32 dev_no,
struct visor_device *from);
diff --git a/drivers/staging/unisys/visorbus/visorchannel.c b/drivers/visorbus/visorchannel.c
similarity index 99%
rename from drivers/staging/unisys/visorbus/visorchannel.c
rename to drivers/visorbus/visorchannel.c
index aae1607..213c4b54 100644
--- a/drivers/staging/unisys/visorbus/visorchannel.c
+++ b/drivers/visorbus/visorchannel.c
@@ -21,8 +21,8 @@
#include <linux/uuid.h>
#include <linux/io.h>
#include <linux/slab.h>
+#include <linux/visorbus/visorbus.h>
-#include "visorbus.h"
#include "visorbus_private.h"
#include "controlvmchannel.h"
diff --git a/drivers/staging/unisys/visorbus/visorchipset.c b/drivers/visorbus/visorchipset.c
similarity index 99%
rename from drivers/staging/unisys/visorbus/visorchipset.c
rename to drivers/visorbus/visorchipset.c
index ef2823a..4ed7386 100644
--- a/drivers/staging/unisys/visorbus/visorchipset.c
+++ b/drivers/visorbus/visorchipset.c
@@ -15,8 +15,8 @@
#include <linux/acpi.h>
#include <linux/crash_dump.h>
+#include <linux/visorbus/visorbus.h>
-#include "visorbus.h"
#include "visorbus_private.h"
/* {72120008-4AAB-11DC-8530-444553544200} */
diff --git a/drivers/staging/unisys/include/visorbus.h b/include/linux/visorbus/visorbus.h
similarity index 100%
rename from drivers/staging/unisys/include/visorbus.h
rename to include/linux/visorbus/visorbus.h
diff --git a/drivers/staging/unisys/include/visorchannel.h b/include/linux/visorbus/visorchannel.h
similarity index 100%
rename from drivers/staging/unisys/include/visorchannel.h
rename to include/linux/visorbus/visorchannel.h
--
1.9.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] drivers: visorbus: move driver out of staging
2017-11-17 17:27 ` [PATCH 2/2] drivers: visorbus: move driver out of staging David Kershner
@ 2017-11-17 19:17 ` Christoph Hellwig
2017-11-17 21:11 ` Kershner, David A
2017-11-18 10:26 ` Greg KH
1 sibling, 1 reply; 8+ messages in thread
From: Christoph Hellwig @ 2017-11-17 19:17 UTC (permalink / raw)
To: David Kershner
Cc: gregkh, jes.sorensen, linux-kernel, driverdev-devel,
sparmaintainer, erik.arfvidson, wadgaonkarsam
Please don' tcreate new subdirectories under include/linux
if you don't have to.
Also who outside of unisys has reviewed this whole code?
Instead of a move please send an actual patchset to add the new files
so people can review it just like any other code.
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH 2/2] drivers: visorbus: move driver out of staging
2017-11-17 19:17 ` Christoph Hellwig
@ 2017-11-17 21:11 ` Kershner, David A
0 siblings, 0 replies; 8+ messages in thread
From: Kershner, David A @ 2017-11-17 21:11 UTC (permalink / raw)
To: Christoph Hellwig
Cc: gregkh, jes.sorensen, linux-kernel, driverdev-devel,
*S-Par-Maintainer, erik.arfvidson, wadgaonkarsam
[-- Attachment #1: Type: text/plain, Size: 1627 bytes --]
> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@infradead.org]
> Sent: Friday, November 17, 2017 2:18 PM
> To: Kershner, David A <David.Kershner@unisys.com>
> Cc: gregkh@linuxfoundation.org; jes.sorensen@gmail.com; linux-
> kernel@vger.kernel.org; driverdev-devel@linuxdriverproject.org; *S-Par-
> Maintainer <SParMaintainer@unisys.com>; erik.arfvidson@gmail.com;
> wadgaonkarsam@gmail.com
> Subject: Re: [PATCH 2/2] drivers: visorbus: move driver out of staging
>
> Please don' tcreate new subdirectories under include/linux
> if you don't have to.
>
Thanks for the feedback, the s-Par drivers have 3 include files in the
include directory in drivers/staging/unisys/include. The patch currently
moves 2 of them, and the third will be moved when the other drivers
get out of staging. When I did the move, I thought one directory with
three files would be cleaner than just adding three files to include. I
will change that.
> Also who outside of unisys has reviewed this whole code?
>
The driver has been in staging for 4 years with significant rework during
that period of time. Throughout that time, we have had input from several
different engineers, including Dan Carpenter, Jes Sorenson, and Greg KH.
In October, I requested a formal review from the community and after the
review had completed, Greg gave us the okay to move them out of staging.
> Instead of a move please send an actual patchset to add the new files
> so people can review it just like any other code.
Okay, I'll redo the patchset to show the explicit add of the files to
the drivers directory.
Thanks,
David Kershner
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 7862 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] drivers: visorbus: move driver out of staging
2017-11-17 17:27 ` [PATCH 2/2] drivers: visorbus: move driver out of staging David Kershner
2017-11-17 19:17 ` Christoph Hellwig
@ 2017-11-18 10:26 ` Greg KH
2017-11-18 14:30 ` Kershner, David A
1 sibling, 1 reply; 8+ messages in thread
From: Greg KH @ 2017-11-18 10:26 UTC (permalink / raw)
To: David Kershner
Cc: jes.sorensen, linux-kernel, driverdev-devel, sparmaintainer,
erik.arfvidson, wadgaonkarsam
On Fri, Nov 17, 2017 at 12:27:39PM -0500, David Kershner wrote:
> {drivers/staging/unisys/include => include/linux/visorbus}/visorbus.h | 0
> .../staging/unisys/include => include/linux/visorbus}/visorchannel.h | 0
Do we really need two different include/linux .h files for this bus
subsystem? Can we merge them both together?
> diff --git a/drivers/staging/unisys/visorbus/Kconfig b/drivers/visorbus/Kconfig
> similarity index 90%
> rename from drivers/staging/unisys/visorbus/Kconfig
> rename to drivers/visorbus/Kconfig
> index 5113880..a99285a 100644
> --- a/drivers/staging/unisys/visorbus/Kconfig
> +++ b/drivers/visorbus/Kconfig
> @@ -4,7 +4,9 @@
>
> config UNISYS_VISORBUS
> tristate "Unisys visorbus driver"
> - depends on UNISYSSPAR
> + depends on X86_64 && !UML
> + select PCI
> + select ACPI
Wait, what? Why are you messing with the dependancies now? And never
do a select if at all possible, do a 'depends on' instead.
And why not UML? What about other arches, why are you now restricting
it? That's an odd change to be doing here in a "move this out of
staging" patch...
thanks,
greg k-h
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH 2/2] drivers: visorbus: move driver out of staging
2017-11-18 10:26 ` Greg KH
@ 2017-11-18 14:30 ` Kershner, David A
0 siblings, 0 replies; 8+ messages in thread
From: Kershner, David A @ 2017-11-18 14:30 UTC (permalink / raw)
To: Greg KH
Cc: jes.sorensen, linux-kernel, driverdev-devel, *S-Par-Maintainer,
erik.arfvidson, wadgaonkarsam
[-- Attachment #1: Type: text/plain, Size: 2077 bytes --]
> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Saturday, November 18, 2017 5:26 AM
> To: Kershner, David A <David.Kershner@unisys.com>
> Cc: jes.sorensen@gmail.com; linux-kernel@vger.kernel.org; driverdev-
> devel@linuxdriverproject.org; *S-Par-Maintainer
> <SParMaintainer@unisys.com>; erik.arfvidson@gmail.com;
> wadgaonkarsam@gmail.com
> Subject: Re: [PATCH 2/2] drivers: visorbus: move driver out of staging
>
> On Fri, Nov 17, 2017 at 12:27:39PM -0500, David Kershner wrote:
> > {drivers/staging/unisys/include => include/linux/visorbus}/visorbus.h |
0
> > .../staging/unisys/include => include/linux/visorbus}/visorchannel.h |
0
>
> Do we really need two different include/linux .h files for this bus
> subsystem? Can we merge them both together?
>
I will merge them together, they were separate so the other channel
definitions
didn't have to include all of visorbus.h for clarity.
> > diff --git a/drivers/staging/unisys/visorbus/Kconfig
> b/drivers/visorbus/Kconfig
> > similarity index 90%
> > rename from drivers/staging/unisys/visorbus/Kconfig
> > rename to drivers/visorbus/Kconfig
> > index 5113880..a99285a 100644
> > --- a/drivers/staging/unisys/visorbus/Kconfig
> > +++ b/drivers/visorbus/Kconfig
> > @@ -4,7 +4,9 @@
> >
> > config UNISYS_VISORBUS
> > tristate "Unisys visorbus driver"
> > - depends on UNISYSSPAR
> > + depends on X86_64 && !UML
> > + select PCI
> > + select ACPI
>
> Wait, what? Why are you messing with the dependancies now? And never
> do a select if at all possible, do a 'depends on' instead.
>
> And why not UML? What about other arches, why are you now restricting
> it? That's an odd change to be doing here in a "move this out of
> staging" patch...
>
When I moved them out of staging, I noticed that it had "depends on
UNISYSSPAR"
flag. The dependencies I added were copied from the UNISYSSPAR dependency
list.
I will make a separate patch to switch them over to depends and determine if
the
!UML flag is still needed.
Thanks,
David Kershner
> thanks,
>
> greg k-h
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 7862 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] staging: unisys: visorbus: address theoretical int overflows
2017-11-17 17:27 ` [PATCH 1/2] staging: unisys: visorbus: address theoretical int overflows David Kershner
@ 2017-11-28 14:11 ` Dan Carpenter
0 siblings, 0 replies; 8+ messages in thread
From: Dan Carpenter @ 2017-11-28 14:11 UTC (permalink / raw)
To: David Kershner
Cc: gregkh, jes.sorensen, linux-kernel, driverdev-devel,
sparmaintainer, erik.arfvidson, wadgaonkarsam, Tim Sell
> --- a/drivers/staging/unisys/visorbus/visorchipset.c
> +++ b/drivers/staging/unisys/visorbus/visorchipset.c
> @@ -581,7 +581,8 @@ static void *parser_name_get(struct parser_context *ctx)
> struct visor_controlvm_parameters_header *phdr;
>
> phdr = &ctx->data;
> - if (phdr->name_offset + phdr->name_length > ctx->param_bytes)
> + if ((unsigned long)phdr->name_offset +
> + (unsigned long)phdr->name_length > ctx->param_bytes)
> return NULL;
> ctx->curr = (char *)&phdr + phdr->name_offset;
> ctx->bytes_remaining = phdr->name_length;
I haven't reviewed this code very thouroughly. This should fix the
issue on 64 bit systems, but it's a no-op on 32 bit systems. Which
might be fine? I would be more comfortable if we just checked for
integer overflow explicitly. There are bunch of ways to do that:
if (phdr->name_offset > ctx->param_bytes ||
phdr->name_length > ctx->param_bytes ||
phdr->name_offset + phdr->name_length > ctx->param_bytes)
Or:
if (phdr->name_offset + phdr->name_length < phdr->name_offset ||
phdr->name_offset + phdr->name_length > ctx->param_bytes)
return NULL;
regards,
dan carpenter
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-11-28 14:12 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-17 17:27 [PATCH 0/2] drivers: Move visorbus from staging to drivers/visorbus David Kershner
2017-11-17 17:27 ` [PATCH 1/2] staging: unisys: visorbus: address theoretical int overflows David Kershner
2017-11-28 14:11 ` Dan Carpenter
2017-11-17 17:27 ` [PATCH 2/2] drivers: visorbus: move driver out of staging David Kershner
2017-11-17 19:17 ` Christoph Hellwig
2017-11-17 21:11 ` Kershner, David A
2017-11-18 10:26 ` Greg KH
2017-11-18 14:30 ` Kershner, David A
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).