Re: [PATCH 3/5] Staging/iio/adc/touchscreen/MXS: add i.MX23 support to the LRADC driver

2013-09-11 Thread Jürgen Beisert

On Tuesday 10 September 2013 22:54:44 Dan Carpenter wrote:
> On Tue, Sep 10, 2013 at 09:36:14AM +0200, Jürgen Beisert wrote:
> > > This patch seems to combine two things -- register access rework AND
> > > adjustment of the driver for MX23 touchscreen.
> >
> > It reworks the register access to avoid the i.MX23/i.MX28 runtime
> > decision all over the code again and again. This is what Dan has
> > recommended in his comment.
>
> I thought the same as Marek that it would be better as two patches like
> the one I sent, but I was also willing to let it slide if no one else
> complained.

Okay, new (re-worked) patch series will follow.

Regards,
Juergen

-- 
Pengutronix e.K.                              | Juergen Beisert             |
Linux Solutions for Science and Industry      | http://www.pengutronix.de/  |
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 3/5] Staging/iio/adc/touchscreen/MXS: add i.MX23 support to the LRADC driver

2013-09-11 Thread Jürgen Beisert

On Tuesday 10 September 2013 23:21:16 Marek Vasut wrote:
> Dear Dan Carpenter,
>
> > On Tue, Sep 10, 2013 at 09:36:14AM +0200, Jürgen Beisert wrote:
> > > > This patch seems to combine two things -- register access rework AND
> > > > adjustment of the driver for MX23 touchscreen.
> > >
> > > It reworks the register access to avoid the i.MX23/i.MX28 runtime
> > > decision all over the code again and again. This is what Dan has
> > > recommended in his comment.
> >
> > I thought the same as Marek that it would be better as two patches like
> > the one I sent, but I was also willing to let it slide if no one else
> > complained.
>
> Looks like Jurgen can now start cursing me ;-)

:))

Regards,
Juergen

-- 
Pengutronix e.K.                              | Juergen Beisert             |
Linux Solutions for Science and Industry      | http://www.pengutronix.de/  |
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 1/1] staging/speakup/kobjects.c: Code improvement.

2013-09-11 Thread Dan Carpenter
On Tue, Sep 10, 2013 at 06:29:39PM -0700, Chris Brannon wrote:
> Ok.  I just sent up a patch to the driverdev list.  I missed a few
> of the Cc's that were on this thread, though.
> Also, it will conflict with Raphael's cleanup.

You're missing Raphael's CC in particular...

Really, Raphael's patch arrived first.  No one seems to have any
objections to his patch.  Normally apply it first and then apply this
one.  If we decide to push this one to -stable then we would redo it (in
other words push the one you just sent).

Of course, the merge window is open right now so nothing is going to be
applied for a couple weeks.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 1/6] Staging/iio/adc/touchscreen/MXS: distinguish i.MX23's and i.MX28's LRADC

2013-09-11 Thread Juergen Beisert
The LRADC units in i.MX23 and i.MX28 differ and we need to distinguish both
SoC variants in order to make the touchscreen work on i.MX23

Signed-off-by: Juergen Beisert 
CC: linux-arm-ker...@lists.infradead.org
CC: de...@driverdev.osuosl.org
CC: Marek Vasut 
CC: Fabio Estevam 
CC: Jonathan Cameron 
---
 drivers/staging/iio/adc/mxs-lradc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/staging/iio/adc/mxs-lradc.c 
b/drivers/staging/iio/adc/mxs-lradc.c
index a08c173..dffca90 100644
--- a/drivers/staging/iio/adc/mxs-lradc.c
+++ b/drivers/staging/iio/adc/mxs-lradc.c
@@ -174,6 +174,8 @@ struct mxs_lradc {
 
struct input_dev*ts_input;
struct work_struct  ts_work;
+
+   enum mxs_lradc_id   soc;
 };
 
 #defineLRADC_CTRL0 0x00
@@ -920,6 +922,7 @@ static int mxs_lradc_probe(struct platform_device *pdev)
}
 
lradc = iio_priv(iio);
+   lradc->soc = (enum mxs_lradc_id)of_id->data;
 
/* Grab the memory area */
iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-- 
1.8.4.rc3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 4/6] Staging/iio/adc/touchscreen/MXS: add i.MX23 support to the LRADC touchscreen driver

2013-09-11 Thread Juergen Beisert
Distinguish i.MX23 and i.MX28 at runtime and do the same for both SoC at least
for the 4 wire touchscreen.

Note: support for the remaining LRADC channels is not tested on an
i.MX23 yet.

Signed-off-by: Juergen Beisert 
CC: linux-arm-ker...@lists.infradead.org
CC: de...@driverdev.osuosl.org
CC: Marek Vasut 
CC: Fabio Estevam 
CC: Jonathan Cameron 
---
 drivers/staging/iio/adc/mxs-lradc.c | 45 +
 1 file changed, 41 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/iio/adc/mxs-lradc.c 
b/drivers/staging/iio/adc/mxs-lradc.c
index 4317fee..c81b186 100644
--- a/drivers/staging/iio/adc/mxs-lradc.c
+++ b/drivers/staging/iio/adc/mxs-lradc.c
@@ -188,20 +188,33 @@ struct mxs_lradc {
 # define LRADC_CTRL0_MX28_XNPSW/* XM */(1 << 17)
 # define LRADC_CTRL0_MX28_XPPSW/* XP */(1 << 16)
 
+# define LRADC_CTRL0_MX23_TOUCH_DETECT_ENABLE  (1 << 20)
+# define LRADC_CTRL0_MX23_YM   (1 << 19)
+# define LRADC_CTRL0_MX23_XM   (1 << 18)
+# define LRADC_CTRL0_MX23_YP   (1 << 17)
+# define LRADC_CTRL0_MX23_XP   (1 << 16)
+
 # define LRADC_CTRL0_MX28_PLATE_MASK \
(LRADC_CTRL0_MX28_TOUCH_DETECT_ENABLE | \
LRADC_CTRL0_MX28_YNNSW | LRADC_CTRL0_MX28_YPNSW | \
LRADC_CTRL0_MX28_YPPSW | LRADC_CTRL0_MX28_XNNSW | \
LRADC_CTRL0_MX28_XNPSW | LRADC_CTRL0_MX28_XPPSW)
 
+# define LRADC_CTRL0_MX23_PLATE_MASK \
+   (LRADC_CTRL0_MX23_TOUCH_DETECT_ENABLE | \
+   LRADC_CTRL0_MX23_YM | LRADC_CTRL0_MX23_XM | \
+   LRADC_CTRL0_MX23_YP | LRADC_CTRL0_MX23_XP)
+
 #defineLRADC_CTRL1 0x10
 #defineLRADC_CTRL1_TOUCH_DETECT_IRQ_EN (1 << 24)
 #defineLRADC_CTRL1_LRADC_IRQ_EN(n) (1 << ((n) + 16))
 #defineLRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK  (0x1fff << 16)
+#defineLRADC_CTRL1_MX23_LRADC_IRQ_EN_MASK  (0x01ff << 16)
 #defineLRADC_CTRL1_LRADC_IRQ_EN_OFFSET 16
 #defineLRADC_CTRL1_TOUCH_DETECT_IRQ(1 << 8)
 #defineLRADC_CTRL1_LRADC_IRQ(n)(1 << (n))
 #defineLRADC_CTRL1_MX28_LRADC_IRQ_MASK 0x1fff
+#defineLRADC_CTRL1_MX23_LRADC_IRQ_MASK 0x01ff
 #defineLRADC_CTRL1_LRADC_IRQ_OFFSET0
 
 #defineLRADC_CTRL2 0x20
@@ -252,36 +265,50 @@ static void mxs_lradc_reg(struct mxs_lradc *lradc, u32 
val, u32 reg)
 
 static u32 mxs_lradc_plate_mask(struct mxs_lradc *lradc)
 {
+   if (lradc->soc == IMX23_LRADC)
+   return LRADC_CTRL0_MX23_PLATE_MASK;
return LRADC_CTRL0_MX28_PLATE_MASK;
 }
 
 static u32 mxs_lradc_irq_en_mask(struct mxs_lradc *lradc)
 {
+   if (lradc->soc == IMX23_LRADC)
+   return LRADC_CTRL1_MX23_LRADC_IRQ_EN_MASK;
return LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK;
 }
 
 static u32 mxs_lradc_irq_mask(struct mxs_lradc *lradc)
 {
+   if (lradc->soc == IMX23_LRADC)
+   return LRADC_CTRL1_MX23_LRADC_IRQ_MASK;
return LRADC_CTRL1_MX28_LRADC_IRQ_MASK;
 }
 
 static u32 mxs_lradc_touch_detect_bit(struct mxs_lradc *lradc)
 {
+   if (lradc->soc == IMX23_LRADC)
+   return LRADC_CTRL0_MX23_TOUCH_DETECT_ENABLE;
return LRADC_CTRL0_MX28_TOUCH_DETECT_ENABLE;
 }
 
 static u32 mxs_lradc_drive_x_plate(struct mxs_lradc *lradc)
 {
+   if (lradc->soc == IMX23_LRADC)
+   return LRADC_CTRL0_MX23_XP | LRADC_CTRL0_MX23_XM;
return LRADC_CTRL0_MX28_XPPSW | LRADC_CTRL0_MX28_XNNSW;
 }
 
 static u32 mxs_lradc_drive_y_plate(struct mxs_lradc *lradc)
 {
+   if (lradc->soc == IMX23_LRADC)
+   return LRADC_CTRL0_MX23_YP | LRADC_CTRL0_MX23_YM;
return LRADC_CTRL0_MX28_YPPSW | LRADC_CTRL0_MX28_YNNSW;
 }
 
 static u32 mxs_lradc_drive_pressure(struct mxs_lradc *lradc)
 {
+   if (lradc->soc == IMX23_LRADC)
+   return LRADC_CTRL0_MX23_YP | LRADC_CTRL0_MX23_XM;
return LRADC_CTRL0_MX28_YPPSW | LRADC_CTRL0_MX28_XNNSW;
 }
 
@@ -319,7 +346,8 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
 * Virtual channel 0 is always used here as the others are always not
 * used if doing raw sampling.
 */
-   mxs_lradc_reg_clear(lradc, LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK,
+   if (lradc->soc == IMX28_LRADC)
+   mxs_lradc_reg_clear(lradc, LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK,
LRADC_CTRL1);
mxs_lradc_reg_clear(lradc, 0xff, LRADC_CTRL0);
 
@@ -767,7 +795,8 @@ static int mxs_lradc_buffer_preenable(struct iio_dev *iio)
if (ret < 0)
goto err_buf;
 
-   mxs_lradc_reg_clear(lradc, LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK,
+   if (lradc->soc == IMX28_LRADC)
+   mxs_lradc_reg_clear(lradc, LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK,
LRADC_CTRL1);
 

[RFCv4] staging/iio/adc: change the MXS touchscreen driver implementation

2013-09-11 Thread Juergen Beisert
The following series replaces the current busy loop touchscreen implementation
for i.MX28/i.MX23 SoCs by a fully interrupt driven implementation.

Since i.MX23 and i.Mx28 silicon differs, the existing implementation can
be used for the i.MX28 SoC only.

So, the first two patches of this series move the i.MX28 specific definitions
out of the way. The third patch simplifies the register access to make it easier
to add the i.MX23 support. Then the i.MX23 specific definitions are added, also
the code to distinguish both SoCs at run-time.
Up to here the existing touchscreen driver will now run on an i.MX23 Soc as 
well.

When the i.MX23 SoC is running from battery it seems not to be a good idea to
run a busy loop to detect touches and their location. The fourth patch adds a
fully interrupt driven implementation which makes use of the built-in delay
and multiple sample features of the touchscreen controller. This will reduce
the interrupt load to a minimum.

The last patch in this series just removes the existing busy loop
implementation.

Still some restrictions/questions:

- the touchscreen part is yet tested on i.MX23 SoC only
- touchscreen parametrisation ability is provided (with fixed values for now)
  but should be done via device tree. Some recommendations how to define the
  bindings would be helpful
- has someone a good idea how to implement a reliable pressure measurement if
  the resistances of the touchscree's plates are unknown?

Changes since v3:

- split adding register access functions and i.MX23 support into two patches

Changes since v2:

- useless debug output removed

Changes since v1:

- adding register access functions to make the existing code more readable
- adding some functions to distinguish the SoCs at run-time to avoid if-else
  contructs whenever differences in the register layout between i.MX23 and
  i.MX28 must be handled

Comments are welcome.

Juergen

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 6/6] Staging/iio/adc/touchscreen/MXS: Remove old touchscreen detection implementation

2013-09-11 Thread Juergen Beisert
Signed-off-by: Juergen Beisert 
CC: linux-arm-ker...@lists.infradead.org
CC: de...@driverdev.osuosl.org
CC: Marek Vasut 
CC: Fabio Estevam 
CC: Jonathan Cameron 
---
 drivers/staging/iio/adc/mxs-lradc.c | 165 
 1 file changed, 165 deletions(-)

diff --git a/drivers/staging/iio/adc/mxs-lradc.c 
b/drivers/staging/iio/adc/mxs-lradc.c
index 185f068..cb791c8 100644
--- a/drivers/staging/iio/adc/mxs-lradc.c
+++ b/drivers/staging/iio/adc/mxs-lradc.c
@@ -183,7 +183,6 @@ struct mxs_lradc {
booluse_touchbutton;
 
struct input_dev*ts_input;
-   struct work_struct  ts_work;
 
enum mxs_lradc_id   soc;
enum lradc_ts_plate cur_plate; /* statemachine */
@@ -820,168 +819,6 @@ static const struct iio_info mxs_lradc_iio_info = {
.read_raw   = mxs_lradc_read_raw,
 };
 
-static int mxs_lradc_ts_touched(struct mxs_lradc *lradc)
-{
-   uint32_t reg;
-
-   /* Enable touch detection. */
-   mxs_lradc_reg_clear(lradc, mxs_lradc_plate_mask(lradc), LRADC_CTRL0);
-   mxs_lradc_reg_set(lradc, mxs_lradc_touch_detect_bit(lradc),
-   LRADC_CTRL0);
-
-   msleep(LRADC_TS_SAMPLE_DELAY_MS);
-
-   reg = readl(lradc->base + LRADC_STATUS);
-
-   return reg & LRADC_STATUS_TOUCH_DETECT_RAW;
-}
-
-static int32_t mxs_lradc_ts_sample(struct mxs_lradc *lradc,
-   enum lradc_ts_plate plate, int change)
-{
-   unsigned long delay, jiff;
-   uint32_t reg, ctrl0 = 0, chan = 0;
-   /* The touchscreen always uses CTRL4 slot #7. */
-   const uint8_t slot = 7;
-   uint32_t val;
-
-   /*
-* There are three correct configurations of the controller sampling
-* the touchscreen, each of these configuration provides different
-* information from the touchscreen.
-*
-* The following table describes the sampling configurations:
-* +-+---+---+---+
-* | Wire \ Axis |   X   |   Y   |   Z   |
-* +-+---+---+
-* |   X+ (CH2)  |   HI  |   TS  |   TS  |
-* +-+---+---+---+
-* |   X- (CH4)  |   LO  |   SH  |   HI  |
-* +-+---+---+---+
-* |   Y+ (CH3)  |   SH  |   HI  |   HI  |
-* +-+---+---+---+
-* |   Y- (CH5)  |   TS  |   LO  |   SH  |
-* +-+---+---+---+
-*
-* HI ... strong '1'  ; LO ... strong '0'
-* SH ... sample here ; TS ... tri-state
-*
-* There are a few other ways of obtaining the Z coordinate
-* (aka. pressure), but the one in the table seems to be the
-* most reliable one.
-*/
-   switch (plate) {
-   case LRADC_SAMPLE_X:
-   ctrl0 = mxs_lradc_drive_x_plate(lradc);
-   chan = 3;
-   break;
-   case LRADC_SAMPLE_Y:
-   ctrl0 = mxs_lradc_drive_y_plate(lradc);
-   chan = 4;
-   break;
-   case LRADC_SAMPLE_PRESSURE:
-   ctrl0 = mxs_lradc_drive_pressure(lradc);
-   chan = 5;
-   break;
-   }
-
-   if (change) {
-   mxs_lradc_reg_clear(lradc, mxs_lradc_plate_mask(lradc),
-   LRADC_CTRL0);
-   mxs_lradc_reg_set(lradc, ctrl0, LRADC_CTRL0);
-
-   mxs_lradc_reg_clear(lradc, LRADC_CTRL4_LRADCSELECT_MASK(slot),
-   LRADC_CTRL4);
-   mxs_lradc_reg_set(lradc,
-   chan << LRADC_CTRL4_LRADCSELECT_OFFSET(slot),
-   LRADC_CTRL4);
-   }
-
-   mxs_lradc_reg_clear(lradc, 0x, LRADC_CH(slot));
-   mxs_lradc_reg_set(lradc, 1 << slot, LRADC_CTRL0);
-
-   delay = jiffies + msecs_to_jiffies(LRADC_TS_SAMPLE_DELAY_MS);
-   do {
-   jiff = jiffies;
-   reg = readl_relaxed(lradc->base + LRADC_CTRL1);
-   if (reg & LRADC_CTRL1_LRADC_IRQ(slot))
-   break;
-   } while (time_before(jiff, delay));
-
-   mxs_lradc_reg_clear(lradc, LRADC_CTRL1_LRADC_IRQ(slot), LRADC_CTRL1);
-
-   if (time_after_eq(jiff, delay))
-   return -ETIMEDOUT;
-
-   val = readl(lradc->base + LRADC_CH(slot));
-   val &= LRADC_CH_VALUE_MASK;
-
-   return val;
-}
-
-static int32_t mxs_lradc_ts_sample_filter(struct mxs_lradc *lradc,
-   enum lradc_ts_plate plate)
-{
-   int32_t val, tot = 0;
-   int i;
-
-   val = mxs_lradc_ts_sample(lradc, plate, 1);
-
-   /* Delay a bit so the touchscreen is stable. */
-   mdelay(2);
-
-   for (i = 0; i < LRADC_TS_SAMPLE_AMOUNT; i++) {
-   val = mxs_lradc_ts_sample(lradc, plate, 0);
-   

[PATCH 3/6] Staging/iio/adc/touchscreen/MXS: simplify register access

2013-09-11 Thread Juergen Beisert
Replace the individual register access by a few shared access function to make 
the
code easier to read and in order to add the i.MX23 SoC in the next step.

Signed-off-by: Juergen Beisert 
CC: linux-arm-ker...@lists.infradead.org
CC: de...@driverdev.osuosl.org
CC: Marek Vasut 
CC: Fabio Estevam 
CC: Jonathan Cameron 
---
 drivers/staging/iio/adc/mxs-lradc.c | 204 +---
 1 file changed, 120 insertions(+), 84 deletions(-)

diff --git a/drivers/staging/iio/adc/mxs-lradc.c 
b/drivers/staging/iio/adc/mxs-lradc.c
index 5535fed..4317fee 100644
--- a/drivers/staging/iio/adc/mxs-lradc.c
+++ b/drivers/staging/iio/adc/mxs-lradc.c
@@ -235,6 +235,56 @@ struct mxs_lradc {
 #define LRADC_RESOLUTION   12
 #define LRADC_SINGLE_SAMPLE_MASK   ((1 << LRADC_RESOLUTION) - 1)
 
+static void mxs_lradc_reg_set(struct mxs_lradc *lradc, u32 val, u32 reg)
+{
+   writel(val, lradc->base + reg + STMP_OFFSET_REG_SET);
+}
+
+static void mxs_lradc_reg_clear(struct mxs_lradc *lradc, u32 val, u32 reg)
+{
+   writel(val, lradc->base + reg + STMP_OFFSET_REG_CLR);
+}
+
+static void mxs_lradc_reg(struct mxs_lradc *lradc, u32 val, u32 reg)
+{
+   writel(val, lradc->base + reg);
+}
+
+static u32 mxs_lradc_plate_mask(struct mxs_lradc *lradc)
+{
+   return LRADC_CTRL0_MX28_PLATE_MASK;
+}
+
+static u32 mxs_lradc_irq_en_mask(struct mxs_lradc *lradc)
+{
+   return LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK;
+}
+
+static u32 mxs_lradc_irq_mask(struct mxs_lradc *lradc)
+{
+   return LRADC_CTRL1_MX28_LRADC_IRQ_MASK;
+}
+
+static u32 mxs_lradc_touch_detect_bit(struct mxs_lradc *lradc)
+{
+   return LRADC_CTRL0_MX28_TOUCH_DETECT_ENABLE;
+}
+
+static u32 mxs_lradc_drive_x_plate(struct mxs_lradc *lradc)
+{
+   return LRADC_CTRL0_MX28_XPPSW | LRADC_CTRL0_MX28_XNNSW;
+}
+
+static u32 mxs_lradc_drive_y_plate(struct mxs_lradc *lradc)
+{
+   return LRADC_CTRL0_MX28_YPPSW | LRADC_CTRL0_MX28_YNNSW;
+}
+
+static u32 mxs_lradc_drive_pressure(struct mxs_lradc *lradc)
+{
+   return LRADC_CTRL0_MX28_YPPSW | LRADC_CTRL0_MX28_XNNSW;
+}
+
 /*
  * Raw I/O operations
  */
@@ -269,21 +319,19 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
 * Virtual channel 0 is always used here as the others are always not
 * used if doing raw sampling.
 */
-   writel(LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK,
-   lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
-   writel(0xff, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
+   mxs_lradc_reg_clear(lradc, LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK,
+   LRADC_CTRL1);
+   mxs_lradc_reg_clear(lradc, 0xff, LRADC_CTRL0);
 
/* Clean the slot's previous content, then set new one. */
-   writel(LRADC_CTRL4_LRADCSELECT_MASK(0),
-   lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_CLR);
-   writel(chan->channel, lradc->base + LRADC_CTRL4 + STMP_OFFSET_REG_SET);
+   mxs_lradc_reg_clear(lradc, LRADC_CTRL4_LRADCSELECT_MASK(0), 
LRADC_CTRL4);
+   mxs_lradc_reg_set(lradc, chan->channel, LRADC_CTRL4);
 
-   writel(0, lradc->base + LRADC_CH(0));
+   mxs_lradc_reg(lradc, 0, LRADC_CH(0));
 
/* Enable the IRQ and start sampling the channel. */
-   writel(LRADC_CTRL1_LRADC_IRQ_EN(0),
-   lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_SET);
-   writel(1 << 0, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
+   mxs_lradc_reg_set(lradc, LRADC_CTRL1_LRADC_IRQ_EN(0), LRADC_CTRL1);
+   mxs_lradc_reg_set(lradc, 1 << 0, LRADC_CTRL0);
 
/* Wait for completion on the channel, 1 second max. */
ret = wait_for_completion_killable_timeout(&lradc->completion, HZ);
@@ -297,8 +345,7 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
ret = IIO_VAL_INT;
 
 err:
-   writel(LRADC_CTRL1_LRADC_IRQ_EN(0),
-   lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
+   mxs_lradc_reg_clear(lradc, LRADC_CTRL1_LRADC_IRQ_EN(0), LRADC_CTRL1);
 
mutex_unlock(&lradc->lock);
 
@@ -324,10 +371,9 @@ static int mxs_lradc_ts_touched(struct mxs_lradc *lradc)
uint32_t reg;
 
/* Enable touch detection. */
-   writel(LRADC_CTRL0_MX28_PLATE_MASK,
-   lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
-   writel(LRADC_CTRL0_MX28_TOUCH_DETECT_ENABLE,
-   lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
+   mxs_lradc_reg_clear(lradc, mxs_lradc_plate_mask(lradc), LRADC_CTRL0);
+   mxs_lradc_reg_set(lradc, mxs_lradc_touch_detect_bit(lradc),
+   LRADC_CTRL0);
 
msleep(LRADC_TS_SAMPLE_DELAY_MS);
 
@@ -372,32 +418,33 @@ static int32_t mxs_lradc_ts_sample(struct mxs_lradc 
*lradc,
 */
switch (plate) {
case LRADC_SAMPLE_X:
-   ctrl0 = LRADC_CTRL0_MX28_XPPSW | LRADC_CTRL0_MX28_XNNSW;
+   ctrl0 = mxs_lradc_drive_x_plate(lradc);
chan = 3;

[PATCH 5/6] Staging/iio/adc/touchscreen/MXS: add interrupt driven touch detection

2013-09-11 Thread Juergen Beisert
For battery driven systems it is a very bad idea to collect the touchscreen
data within a kernel busy loop.

This change uses the features of the hardware to delay and accumulate samples in
hardware to avoid a high interrupt and CPU load.

Note: this is only tested on an i.MX23 SoC yet.

Signed-off-by: Juergen Beisert 
CC: linux-arm-ker...@lists.infradead.org
CC: de...@driverdev.osuosl.org
CC: Marek Vasut 
CC: Fabio Estevam 
CC: Jonathan Cameron 
---
 drivers/staging/iio/adc/mxs-lradc.c | 530 
 1 file changed, 476 insertions(+), 54 deletions(-)

diff --git a/drivers/staging/iio/adc/mxs-lradc.c 
b/drivers/staging/iio/adc/mxs-lradc.c
index c81b186..185f068 100644
--- a/drivers/staging/iio/adc/mxs-lradc.c
+++ b/drivers/staging/iio/adc/mxs-lradc.c
@@ -129,6 +129,17 @@ enum mxs_lradc_ts {
MXS_LRADC_TOUCHSCREEN_5WIRE,
 };
 
+/*
+ * Touchscreen handling
+ */
+enum lradc_ts_plate {
+   LRADC_TOUCH = 0,
+   LRADC_SAMPLE_X,
+   LRADC_SAMPLE_Y,
+   LRADC_SAMPLE_PRESSURE,
+   LRADC_SAMPLE_VALID,
+};
+
 struct mxs_lradc {
struct device   *dev;
void __iomem*base;
@@ -169,13 +180,25 @@ struct mxs_lradc {
 #define CHAN_MASK_TOUCHSCREEN_4WIRE(0xf << 2)
 #define CHAN_MASK_TOUCHSCREEN_5WIRE(0x1f << 2)
enum mxs_lradc_ts   use_touchscreen;
-   boolstop_touchscreen;
booluse_touchbutton;
 
struct input_dev*ts_input;
struct work_struct  ts_work;
 
enum mxs_lradc_id   soc;
+   enum lradc_ts_plate cur_plate; /* statemachine */
+   boolts_valid;
+   unsignedts_x_pos;
+   unsignedts_y_pos;
+   unsignedts_pressure;
+
+   /* handle touchscreen's physical behaviour */
+   /* samples per coordinate */
+   unsignedover_sample_cnt;
+   /* time clocks between samples */
+   unsignedover_sample_delay;
+   /* time in clocks to wait after the plates where switched */
+   unsignedsettling_delay;
 };
 
 #defineLRADC_CTRL0 0x00
@@ -227,19 +250,33 @@ struct mxs_lradc {
 #defineLRADC_CH_ACCUMULATE (1 << 29)
 #defineLRADC_CH_NUM_SAMPLES_MASK   (0x1f << 24)
 #defineLRADC_CH_NUM_SAMPLES_OFFSET 24
+#defineLRADC_CH_NUM_SAMPLES(x) \
+   ((x) << LRADC_CH_NUM_SAMPLES_OFFSET)
 #defineLRADC_CH_VALUE_MASK 0x3
 #defineLRADC_CH_VALUE_OFFSET   0
 
 #defineLRADC_DELAY(n)  (0xd0 + (0x10 * (n)))
 #defineLRADC_DELAY_TRIGGER_LRADCS_MASK (0xff << 24)
 #defineLRADC_DELAY_TRIGGER_LRADCS_OFFSET   24
+#defineLRADC_DELAY_TRIGGER(x) \
+   (((x) << LRADC_DELAY_TRIGGER_LRADCS_OFFSET) & \
+   LRADC_DELAY_TRIGGER_LRADCS_MASK)
 #defineLRADC_DELAY_KICK(1 << 20)
 #defineLRADC_DELAY_TRIGGER_DELAYS_MASK (0xf << 16)
 #defineLRADC_DELAY_TRIGGER_DELAYS_OFFSET   16
+#defineLRADC_DELAY_TRIGGER_DELAYS(x) \
+   (((x) << LRADC_DELAY_TRIGGER_DELAYS_OFFSET) & \
+   LRADC_DELAY_TRIGGER_DELAYS_MASK)
 #defineLRADC_DELAY_LOOP_COUNT_MASK (0x1f << 11)
 #defineLRADC_DELAY_LOOP_COUNT_OFFSET   11
+#defineLRADC_DELAY_LOOP(x) \
+   (((x) << LRADC_DELAY_LOOP_COUNT_OFFSET) & \
+   LRADC_DELAY_LOOP_COUNT_MASK)
 #defineLRADC_DELAY_DELAY_MASK  0x7ff
 #defineLRADC_DELAY_DELAY_OFFSET0
+#defineLRADC_DELAY_DELAY(x) \
+   (((x) << LRADC_DELAY_DELAY_OFFSET) & \
+   LRADC_DELAY_DELAY_MASK)
 
 #defineLRADC_CTRL4 0x140
 #defineLRADC_CTRL4_LRADCSELECT_MASK(n) (0xf << ((n) * 4))
@@ -312,6 +349,404 @@ static u32 mxs_lradc_drive_pressure(struct mxs_lradc 
*lradc)
return LRADC_CTRL0_MX28_YPPSW | LRADC_CTRL0_MX28_XNNSW;
 }
 
+static bool mxs_lradc_check_touch_event(struct mxs_lradc *lradc)
+{
+   return !!(readl(lradc->base + LRADC_STATUS) &
+   LRADC_STATUS_TOUCH_DETECT_RAW);
+}
+
+static void mxs_lradc_setup_ts_channel(struct mxs_lradc *lradc, unsigned ch)
+{
+   /*
+* prepare for oversampling conversion
+*
+* from the datasheet:
+* "The ACCUMULATE bit in the appropriate channel register
+* HW_LRADC_CHn must be set to 1 if NUM_SAMPLES is greater then 0;
+* otherwise, the IRQs will not fire."
+*/
+   mxs_lradc_reg(lradc, LRADC_CH_ACCUMULATE |
+ 

[PATCH 2/6] Staging/iio/adc/touchscreen/MXS: separate i.MX28 specific register bits

2013-09-11 Thread Juergen Beisert
In order to support i.MX23 and i.MX28 within one driver we need to separate the
register definitions which differ in both SoC variants.

Signed-off-by: Juergen Beisert 
CC: linux-arm-ker...@lists.infradead.org
CC: de...@driverdev.osuosl.org
CC: Marek Vasut 
CC: Fabio Estevam 
CC: Jonathan Cameron 
---
 drivers/staging/iio/adc/mxs-lradc.c | 61 -
 1 file changed, 33 insertions(+), 28 deletions(-)

diff --git a/drivers/staging/iio/adc/mxs-lradc.c 
b/drivers/staging/iio/adc/mxs-lradc.c
index dffca90..5535fed 100644
--- a/drivers/staging/iio/adc/mxs-lradc.c
+++ b/drivers/staging/iio/adc/mxs-lradc.c
@@ -179,24 +179,29 @@ struct mxs_lradc {
 };
 
 #defineLRADC_CTRL0 0x00
-#defineLRADC_CTRL0_TOUCH_DETECT_ENABLE (1 << 23)
-#defineLRADC_CTRL0_TOUCH_SCREEN_TYPE   (1 << 22)
-#defineLRADC_CTRL0_YNNSW   /* YM */(1 << 21)
-#defineLRADC_CTRL0_YPNSW   /* YP */(1 << 20)
-#defineLRADC_CTRL0_YPPSW   /* YP */(1 << 19)
-#defineLRADC_CTRL0_XNNSW   /* XM */(1 << 18)
-#defineLRADC_CTRL0_XNPSW   /* XM */(1 << 17)
-#defineLRADC_CTRL0_XPPSW   /* XP */(1 << 16)
-#defineLRADC_CTRL0_PLATE_MASK  (0x3f << 16)
+# define LRADC_CTRL0_MX28_TOUCH_DETECT_ENABLE  (1 << 23)
+# define LRADC_CTRL0_MX28_TOUCH_SCREEN_TYPE(1 << 22)
+# define LRADC_CTRL0_MX28_YNNSW/* YM */(1 << 21)
+# define LRADC_CTRL0_MX28_YPNSW/* YP */(1 << 20)
+# define LRADC_CTRL0_MX28_YPPSW/* YP */(1 << 19)
+# define LRADC_CTRL0_MX28_XNNSW/* XM */(1 << 18)
+# define LRADC_CTRL0_MX28_XNPSW/* XM */(1 << 17)
+# define LRADC_CTRL0_MX28_XPPSW/* XP */(1 << 16)
+
+# define LRADC_CTRL0_MX28_PLATE_MASK \
+   (LRADC_CTRL0_MX28_TOUCH_DETECT_ENABLE | \
+   LRADC_CTRL0_MX28_YNNSW | LRADC_CTRL0_MX28_YPNSW | \
+   LRADC_CTRL0_MX28_YPPSW | LRADC_CTRL0_MX28_XNNSW | \
+   LRADC_CTRL0_MX28_XNPSW | LRADC_CTRL0_MX28_XPPSW)
 
 #defineLRADC_CTRL1 0x10
 #defineLRADC_CTRL1_TOUCH_DETECT_IRQ_EN (1 << 24)
 #defineLRADC_CTRL1_LRADC_IRQ_EN(n) (1 << ((n) + 16))
-#defineLRADC_CTRL1_LRADC_IRQ_EN_MASK   (0x1fff << 16)
+#defineLRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK  (0x1fff << 16)
 #defineLRADC_CTRL1_LRADC_IRQ_EN_OFFSET 16
 #defineLRADC_CTRL1_TOUCH_DETECT_IRQ(1 << 8)
 #defineLRADC_CTRL1_LRADC_IRQ(n)(1 << (n))
-#defineLRADC_CTRL1_LRADC_IRQ_MASK  0x1fff
+#defineLRADC_CTRL1_MX28_LRADC_IRQ_MASK 0x1fff
 #defineLRADC_CTRL1_LRADC_IRQ_OFFSET0
 
 #defineLRADC_CTRL2 0x20
@@ -264,7 +269,7 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
 * Virtual channel 0 is always used here as the others are always not
 * used if doing raw sampling.
 */
-   writel(LRADC_CTRL1_LRADC_IRQ_EN_MASK,
+   writel(LRADC_CTRL1_MX28_LRADC_IRQ_EN_MASK,
lradc->base + LRADC_CTRL1 + STMP_OFFSET_REG_CLR);
writel(0xff, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
 
@@ -319,9 +324,9 @@ static int mxs_lradc_ts_touched(struct mxs_lradc *lradc)
uint32_t reg;
 
/* Enable touch detection. */
-   writel(LRADC_CTRL0_PLATE_MASK,
+   writel(LRADC_CTRL0_MX28_PLATE_MASK,
lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
-   writel(LRADC_CTRL0_TOUCH_DETECT_ENABLE,
+   writel(LRADC_CTRL0_MX28_TOUCH_DETECT_ENABLE,
lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
 
msleep(LRADC_TS_SAMPLE_DELAY_MS);
@@ -367,21 +372,21 @@ static int32_t mxs_lradc_ts_sample(struct mxs_lradc 
*lradc,
 */
switch (plate) {
case LRADC_SAMPLE_X:
-   ctrl0 = LRADC_CTRL0_XPPSW | LRADC_CTRL0_XNNSW;
+   ctrl0 = LRADC_CTRL0_MX28_XPPSW | LRADC_CTRL0_MX28_XNNSW;
chan = 3;
break;
case LRADC_SAMPLE_Y:
-   ctrl0 = LRADC_CTRL0_YPPSW | LRADC_CTRL0_YNNSW;
+   ctrl0 = LRADC_CTRL0_MX28_YPPSW | LRADC_CTRL0_MX28_YNNSW;
chan = 4;
break;
case LRADC_SAMPLE_PRESSURE:
-   ctrl0 = LRADC_CTRL0_YPPSW | LRADC_CTRL0_XNNSW;
+   ctrl0 = LRADC_CTRL0_MX28_YPPSW | LRADC_CTRL0_MX28_XNNSW;
chan = 5;
break;
}
 
if (change) {
-   writel(LRADC_CTRL0_PLATE_MASK,
+   writel(LRADC_CTRL0_MX28_PLATE_MASK,
lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_CLR);
writel(ctrl0, lradc->base + LRADC_CTRL0 + STMP_OFFSET_REG_SET);
 
@@ -442,7 +447,7 @@ static void mxs_lradc_ts_work(struct work_st

Re: [HELP] mediabuttons drivers on MSI GE60 laptop

2013-09-11 Thread Dan Carpenter
On Wed, Sep 11, 2013 at 10:46:37AM +0700, Vadim A. Misbakh-Soloviov wrote:
>
> P.S. is it correct maillist for such help requests at all? :)
> 

You would probably have better luck asking on
linux-in...@vger.kernel.org.

Specify exactly which laptop you are using, btw.  It may be a known
thing.  Also have you tried enabling the platform code under
drivers/platform/x86/your_laptop.c?

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: dgnc: fix potential format string flaw

2013-09-11 Thread Joe Perches
On Tue, 2013-09-10 at 21:41 -0700, Kees Cook wrote:
> Make sure that format strings cannot leak into printk() calls from the
> msgbuf string.

printf(string);
vs
printf("%s", string);

How does this help?


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [HELP] mediabuttons drivers on MSI GE60 laptop

2013-09-11 Thread Vadim A. Misbakh-Soloviov

Hi, Dan!
11.09.2013 15:30, Dan Carpenter пишет:
> On Wed, Sep 11, 2013 at 10:46:37AM +0700, Vadim A. Misbakh-Soloviov wrote:
>>
>> P.S. is it correct maillist for such help requests at all? :)
>>
> 
> You would probably have better luck asking on
> linux-in...@vger.kernel.org.

Can I just add it as CC, or should I rewrite there from the scratch?

> Specify exactly which laptop you are using, btw.  It may be a known
> thing. 

There, or here? Anyway, MSI GE60 0NC

> Also have you tried enabling the platform code under
> drivers/platform/x86/your_laptop.c?

MSI extras? Yup, I tried it, but absolutely without any effect.

> regards,
> dan carpenter
> 



signature.asc
Description: OpenPGP digital signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [HELP] mediabuttons drivers on MSI GE60 laptop

2013-09-11 Thread Dan Carpenter
On Wed, Sep 11, 2013 at 03:34:51PM +0700, Vadim A. Misbakh-Soloviov wrote:
> > You would probably have better luck asking on
> > linux-in...@vger.kernel.org.
> 
> Can I just add it as CC, or should I rewrite there from the scratch?
> 

Send it to them and drop us from the list.  This list has a broad
mandate so we don't complain about offtopic emails but we don't have
the experts so we're not very useful.  ;)

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: dgnc: fix potential format string flaw

2013-09-11 Thread Dan Carpenter
On Tue, Sep 10, 2013 at 10:19:17PM -0700, Kees Cook wrote:
> In the former case, format characters will get processed by the
> sprintf logic. In the latter, they are printed as-is. In this specific
> case, if there was a way to inject strings like "ohai %n" into the
> msgbuf string, the former would actually attempt to resolve the %n. In
> the simple case, this could lead to Oopses, and in the unlucky case,
> it could allow arbitrary memory writing and execution control.
> 
> http://en.wikipedia.org/wiki/Uncontrolled_format_string

The kernel ignores %n so hopefully it can't actually write to memory.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] Drivers: hv: vmbus: fix error return code in vmbus_connect()

2013-09-11 Thread Wei Yongjun
From: Wei Yongjun 

Fix to return -EINVAL in the version check error handling
case instead of 0, as done elsewhere in this function.

Signed-off-by: Wei Yongjun 
---
 drivers/hv/connection.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index 8f4743a..b3eb50f 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -204,8 +204,10 @@ int vmbus_connect(void)
version = vmbus_get_next_version(version);
} while (version != VERSION_INVAL);
 
-   if (version == VERSION_INVAL)
+   if (version == VERSION_INVAL) {
+   ret = -EINVAL;
goto cleanup;
+   }
 
vmbus_proto_version = version;
pr_info("Hyper-V Host Build:%d-%d.%d-%d-%d.%d; Vmbus version:%d.%d\n",

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 3/4] staging: et131x: Whitespace changes, cat some spilt lines

2013-09-11 Thread Mark Einon
On Wed, Sep 04, 2013 at 12:19:16PM +0300, Dan Carpenter wrote:
> Sorry, this is still in my postponed messages folder.  I meant to send
> it earlier.
> 
> On Mon, Sep 02, 2013 at 10:23:21PM +0100, Mark Einon wrote:
> > Ignoring checkpatch for some lines - now just over 80 chars, but much
> > more readable.
> > 
> 
> The person who "fixed" these long lines, did it in the wrong way, yes.
> But we always apply those because it's the easiest way and you can't
> fight against checkpatch.pl.  If we go back to long lines, we'll just
> immediately apply another "break the long lines up" patch from some
> newbie who doesn't know any better.
> 
> We need to fix it in the right way instead of re-introducing checkpatch
> warnings.

** Greg ** - please drop this patchset, I will re-send the other three
without this one.

Hi Dan,

I understand your point, thanks for the review.
As I've little time to make a large change at this time, I'll add an
item to the TODO based on your points above.

Cheers,

Mark
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 3/4] staging: et131x: Whitespace changes, cat some spilt lines

2013-09-11 Thread Dan Carpenter
On Wed, Sep 11, 2013 at 01:29:49PM +0100, Mark Einon wrote:
> On Wed, Sep 04, 2013 at 12:19:16PM +0300, Dan Carpenter wrote:
> > Sorry, this is still in my postponed messages folder.  I meant to send
> > it earlier.
> > 
> > On Mon, Sep 02, 2013 at 10:23:21PM +0100, Mark Einon wrote:
> > > Ignoring checkpatch for some lines - now just over 80 chars, but much
> > > more readable.
> > > 
> > 
> > The person who "fixed" these long lines, did it in the wrong way, yes.
> > But we always apply those because it's the easiest way and you can't
> > fight against checkpatch.pl.  If we go back to long lines, we'll just
> > immediately apply another "break the long lines up" patch from some
> > newbie who doesn't know any better.
> > 
> > We need to fix it in the right way instead of re-introducing checkpatch
> > warnings.
> 
> ** Greg ** - please drop this patchset, I will re-send the other three
> without this one.
> 

The other patches can apply without this one although the line
numbers are shifted for [PATCH 4/4 v2].

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 3/4 RESEND] staging: et131x: Removing some unecessary braces

2013-09-11 Thread Mark Einon
Remove braces from a few single line if statements.

Signed-off-by: Mark Einon 
---
 drivers/staging/et131x/et131x.c |   12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/et131x/et131x.c b/drivers/staging/et131x/et131x.c
index a8f7cd7..7cd3d34 100644
--- a/drivers/staging/et131x/et131x.c
+++ b/drivers/staging/et131x/et131x.c
@@ -2774,10 +2774,9 @@ static void et131x_handle_recv_interrupt(struct 
et131x_adapter *adapter)
adapter->net_stats.rx_packets++;
 
/* Set the status on the packet, either resources or success */
-   if (adapter->rx_ring.num_ready_recv < RFD_LOW_WATER_MARK) {
-   dev_warn(&adapter->pdev->dev,
-   "RFD's are running out\n");
-   }
+   if (adapter->rx_ring.num_ready_recv < RFD_LOW_WATER_MARK)
+   dev_warn(&adapter->pdev->dev, "RFD's are running 
out\n");
+
count++;
}
 
@@ -3097,11 +3096,10 @@ static int send_packet(struct sk_buff *skb, struct 
et131x_adapter *adapter)
shbufva = (u16 *) skb->data;
 
if ((shbufva[0] == 0x) &&
-   (shbufva[1] == 0x) && (shbufva[2] == 0x)) {
+   (shbufva[1] == 0x) && (shbufva[2] == 0x))
tcb->flags |= FMP_DEST_BROAD;
-   } else if ((shbufva[0] & 0x3) == 0x0001) {
+   else if ((shbufva[0] & 0x3) == 0x0001)
tcb->flags |=  FMP_DEST_MULTI;
-   }
}
 
tcb->next = NULL;
-- 
1.7.10.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 1/4 RESEND] staging: et131x: Remove unused spinlock

2013-09-11 Thread Mark Einon
phy_lock is no longer used in any useful code, it's all been moved into
a phy_device. Remove the lock definition and init.

Signed-off-by: Mark Einon 
---
 drivers/staging/et131x/et131x.c |3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/staging/et131x/et131x.c b/drivers/staging/et131x/et131x.c
index f73e58f..4c1eade 100644
--- a/drivers/staging/et131x/et131x.c
+++ b/drivers/staging/et131x/et131x.c
@@ -496,8 +496,6 @@ struct et131x_adapter {
spinlock_t rcv_pend_lock;
spinlock_t fbr_lock;
 
-   spinlock_t phy_lock;
-
/* Packet Filter and look ahead size */
u32 packet_filter;
 
@@ -3928,7 +3926,6 @@ static struct et131x_adapter *et131x_adapter_init(struct 
net_device *netdev,
spin_lock_init(&adapter->rcv_lock);
spin_lock_init(&adapter->rcv_pend_lock);
spin_lock_init(&adapter->fbr_lock);
-   spin_lock_init(&adapter->phy_lock);
 
adapter->registry_jumbo_packet = 1514;  /* 1514-9216 */
 
-- 
1.7.10.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 4/4] staging: et131x: Add some items to the TODO list

2013-09-11 Thread Mark Einon
Adding some trivial formatting suggestions made by Dan Carpenter to
the TODO.

Signed-off-by: Mark Einon 
---
 drivers/staging/et131x/README |7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/et131x/README b/drivers/staging/et131x/README
index 9272a24..8da96a6 100644
--- a/drivers/staging/et131x/README
+++ b/drivers/staging/et131x/README
@@ -11,7 +11,12 @@ TODO:
- Look at reducing the number of spinlocks
- Simplify code in nic_rx_pkts(), when determining multicast_pkts_rcvd
- Implement NAPI support
-   - in et131x_tx(), don't return NETDEV_TX_BUSY, just drop the packet 
with  kfree_skb().
+   - In et131x_tx(), don't return NETDEV_TX_BUSY, just drop the packet 
with kfree_skb().
+   - Reduce the number of split lines by careful consideration of variable 
names etc.
+   - Do this in et131x.c:
+struct fbr_lookup *fbr;
+fbr = rx_local->fbr[id];
+ Then replace all the instances of "rx_local->fbr[id]" with fbr.
 
 Please send patches to:
Greg Kroah-Hartman 
-- 
1.7.10.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 2/4 RESEND] staging: et131x: Remove unused rcv_pend_lock spinlock

2013-09-11 Thread Mark Einon
The rcv_pend_lock spinlock isn't used anymore. remove it.

Signed-off-by: Mark Einon 
---
 drivers/staging/et131x/et131x.c |2 --
 1 file changed, 2 deletions(-)
 create mode 100644 drivers/staging/et131x/Module.symvers

diff --git a/drivers/staging/et131x/Module.symvers 
b/drivers/staging/et131x/Module.symvers
new file mode 100644
index 000..e69de29
diff --git a/drivers/staging/et131x/et131x.c b/drivers/staging/et131x/et131x.c
index 4c1eade..a8f7cd7 100644
--- a/drivers/staging/et131x/et131x.c
+++ b/drivers/staging/et131x/et131x.c
@@ -493,7 +493,6 @@ struct et131x_adapter {
spinlock_t send_hw_lock;
 
spinlock_t rcv_lock;
-   spinlock_t rcv_pend_lock;
spinlock_t fbr_lock;
 
/* Packet Filter and look ahead size */
@@ -3924,7 +3923,6 @@ static struct et131x_adapter *et131x_adapter_init(struct 
net_device *netdev,
spin_lock_init(&adapter->tcb_ready_qlock);
spin_lock_init(&adapter->send_hw_lock);
spin_lock_init(&adapter->rcv_lock);
-   spin_lock_init(&adapter->rcv_pend_lock);
spin_lock_init(&adapter->fbr_lock);
 
adapter->registry_jumbo_packet = 1514;  /* 1514-9216 */
-- 
1.7.10.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 02/11] staging: usbip: Add support for client authentication

2013-09-11 Thread Dominik Paulus
This patch adds support for authenticating both client and server using
a pre-shared passphrase using SRP (Secure Remote Password) over TLS (see
RFC 5054) using GnuTLS. Both usbip and usbipd now accept a shared secret
as a command line argument. Currently, the established TLS connection is
only used to perform a secure handshake and dropped before the socket is
passed to the kernel. The code may be extended to exchange a session key
over TLS and pass it to the kernel to perform IPsec.

Signed-off-by: Dominik Paulus 
Signed-off-by: Tobias Polzer 
---
 drivers/staging/usbip/userspace/configure.ac   |  14 ++
 drivers/staging/usbip/userspace/doc/usbip.8|   6 +
 drivers/staging/usbip/userspace/doc/usbipd.8   |   6 +
 drivers/staging/usbip/userspace/src/usbip.c|  30 ++-
 drivers/staging/usbip/userspace/src/usbip_attach.c |   2 +-
 drivers/staging/usbip/userspace/src/usbip_list.c   |   2 +-
 .../staging/usbip/userspace/src/usbip_network.c|  82 
 .../staging/usbip/userspace/src/usbip_network.h|   9 +-
 drivers/staging/usbip/userspace/src/usbipd.c   | 217 ++---
 9 files changed, 335 insertions(+), 33 deletions(-)

diff --git a/drivers/staging/usbip/userspace/configure.ac 
b/drivers/staging/usbip/userspace/configure.ac
index 2be4060..7bba496 100644
--- a/drivers/staging/usbip/userspace/configure.ac
+++ b/drivers/staging/usbip/userspace/configure.ac
@@ -84,6 +84,20 @@ AC_ARG_WITH([tcp-wrappers],
AC_DEFINE([HAVE_LIBWRAP], [1], [use tcp wrapper])],
   [AC_MSG_RESULT([no]); LIBS="$saved_LIBS"])])
 
+# Checks for the GnuTLS library
+AC_ARG_WITH([gnutls],
+   [AS_HELP_STRING([--with-gnutls],
+   [use the GnuTLS library 
for authentication])],
+   dnl [ACTION-IF-GIVEN]
+   [if test "$withval" = "yes"; then
+PKG_CHECK_MODULES([GNUTLS], [gnutls])
+AC_DEFINE([HAVE_GNUTLS], [1], [use gnutls])
+CFLAGS="$CFLAGS $GNUTLS_CFLAGS"
+LDFLAGS="$LDFLAGS $GNUTLS_LIBS"
+fi
+   ],
+   )
+
 # Sets directory containing usb.ids.
 AC_ARG_WITH([usbids-dir],
[AS_HELP_STRING([--with-usbids-dir=DIR],
diff --git a/drivers/staging/usbip/userspace/doc/usbip.8 
b/drivers/staging/usbip/userspace/doc/usbip.8
index a6097be..b5050ed 100644
--- a/drivers/staging/usbip/userspace/doc/usbip.8
+++ b/drivers/staging/usbip/userspace/doc/usbip.8
@@ -29,6 +29,12 @@ Log to syslog.
 Connect to PORT on remote host (used for attach and list --remote).
 .PP
 
+.HP
+\fB\-\-auth\fR
+.IP
+Set the password to be used for client authentication. See usbipd(8) for more 
information.
+.PP
+
 .SH COMMANDS
 .HP
 \fBversion\fR
diff --git a/drivers/staging/usbip/userspace/doc/usbipd.8 
b/drivers/staging/usbip/userspace/doc/usbipd.8
index ac4635d..b2b9eee 100644
--- a/drivers/staging/usbip/userspace/doc/usbipd.8
+++ b/drivers/staging/usbip/userspace/doc/usbipd.8
@@ -54,6 +54,12 @@ If no FILE specified, use /var/run/usbipd.pid
 Listen on TCP/IP port PORT.
 .PP
 
+.HP
+\fB\-s\fR, \fB\-\-auth\fR
+.IP
+Sets the password to be used for client authentication. If -a is used, the 
server will only accept connections from authenticated clients. Note: USB 
traffic will still be unencrypted, this currently only serves for 
authentication.
+.PP
+
 \fB\-h\fR, \fB\-\-help\fR
 .IP
 Print the program help message and exit.
diff --git a/drivers/staging/usbip/userspace/src/usbip.c 
b/drivers/staging/usbip/userspace/src/usbip.c
index 04a5f20..8a5de83 100644
--- a/drivers/staging/usbip/userspace/src/usbip.c
+++ b/drivers/staging/usbip/userspace/src/usbip.c
@@ -25,6 +25,12 @@
 #include 
 #include 
 
+#include "../config.h"
+
+#ifdef HAVE_GNUTLS
+#include 
+#endif
+
 #include "usbip_common.h"
 #include "usbip_network.h"
 #include "usbip.h"
@@ -35,8 +41,12 @@ static int usbip_version(int argc, char *argv[]);
 static const char usbip_version_string[] = PACKAGE_STRING;
 
 static const char usbip_usage_string[] =
-   "usbip [--debug] [--log] [--tcp-port PORT] [version]\n"
-   " [help]  \n";
+   "usbip "
+#ifdef HAVE_GNUTLS
+   "[--auth PASSWORD] "
+#endif
+   "[--debug] [--log] [--tcp-port PORT]\n"
+   " [version] [help]  \n";
 
 static void usbip_usage(void)
 {
@@ -142,6 +152,7 @@ int main(int argc, char *argv[])
{ "debug",no_argument,   NULL, 'd' },
{ "log",  no_argument,   NULL, 'l' },
{ "tcp-port", required_argument, NULL, 't' },
+   { "auth", required_argument, NULL, 's' },
{ NULL,   0, NULL,  0  }
};
 
@@ -152,12 +163,25 @@ int main(int argc, char *argv[])
usbip_use_stderr = 1;
opterr = 0;
for (;;) {
-   opt = getopt_long(argc, argv

[PATCH 11/11] staging: usbip: Increment version to 1.2.0

2013-09-11 Thread Dominik Paulus
Signed-off-by: Dominik Paulus 
Signed-off-by: Tobias Polzer 
---
 drivers/staging/usbip/userspace/configure.ac | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/usbip/userspace/configure.ac 
b/drivers/staging/usbip/userspace/configure.ac
index 099d24b..0b0e035 100644
--- a/drivers/staging/usbip/userspace/configure.ac
+++ b/drivers/staging/usbip/userspace/configure.ac
@@ -1,7 +1,7 @@
 dnl Process this file with autoconf to produce a configure script.
 
 AC_PREREQ(2.59)
-AC_INIT([usbip-utils], [1.1.1], [linux-...@vger.kernel.org])
+AC_INIT([usbip-utils], [1.2.0], [linux-...@vger.kernel.org])
 
 CURRENT=0
 REVISION=1
-- 
1.8.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 01/11] staging: usbip: Fix IPv6 support in usbipd

2013-09-11 Thread Dominik Paulus
getaddrinfo() leaves the order of the returned addrinfo structs
unspecified. On systems with bindv6only disabled (this is the default),
PF_INET6 sockets bind to IPv4, too. Thus, IPv6 support in usbipd was
broken when getaddrinfo returned first IPv4 and then IPv6 addrinfos, as
the IPv6 bind failed with EADDRINUSE.

This patch uses seperate sockets for IPv4 and IPv6 and sets IPV6_V6ONLY
on all IPv6 sockets. Two command line arguments, -4 and -6 were added to
manually select the socket family.

Signed-off-by: Tobias Polzer 
Signed-off-by: Dominik Paulus 
---
 .../staging/usbip/userspace/src/usbip_network.c| 12 
 .../staging/usbip/userspace/src/usbip_network.h|  1 +
 drivers/staging/usbip/userspace/src/usbipd.c   | 69 --
 3 files changed, 64 insertions(+), 18 deletions(-)

diff --git a/drivers/staging/usbip/userspace/src/usbip_network.c 
b/drivers/staging/usbip/userspace/src/usbip_network.c
index c39a07f..e78279c 100644
--- a/drivers/staging/usbip/userspace/src/usbip_network.c
+++ b/drivers/staging/usbip/userspace/src/usbip_network.c
@@ -239,6 +239,18 @@ int usbip_net_set_keepalive(int sockfd)
return ret;
 }
 
+int usbip_net_set_v6only(int sockfd)
+{
+   const int val = 1;
+   int ret;
+
+   ret = setsockopt(sockfd, IPPROTO_IPV6, IPV6_V6ONLY, &val, sizeof(val));
+   if (ret < 0)
+   dbg("setsockopt: IPV6_V6ONLY");
+
+   return ret;
+}
+
 /*
  * IPv6 Ready
  */
diff --git a/drivers/staging/usbip/userspace/src/usbip_network.h 
b/drivers/staging/usbip/userspace/src/usbip_network.h
index 2d0e427..f19ae19 100644
--- a/drivers/staging/usbip/userspace/src/usbip_network.h
+++ b/drivers/staging/usbip/userspace/src/usbip_network.h
@@ -180,6 +180,7 @@ int usbip_net_recv_op_common(int sockfd, uint16_t *code);
 int usbip_net_set_reuseaddr(int sockfd);
 int usbip_net_set_nodelay(int sockfd);
 int usbip_net_set_keepalive(int sockfd);
+int usbip_net_set_v6only(int sockfd);
 int usbip_net_tcp_connect(char *hostname, char *port);
 
 #endif /* __USBIP_NETWORK_H */
diff --git a/drivers/staging/usbip/userspace/src/usbipd.c 
b/drivers/staging/usbip/userspace/src/usbipd.c
index 1c76cfd..7980f8b 100644
--- a/drivers/staging/usbip/userspace/src/usbipd.c
+++ b/drivers/staging/usbip/userspace/src/usbipd.c
@@ -56,6 +56,13 @@ static const char usbip_version_string[] = PACKAGE_STRING;
 
 static const char usbipd_help_string[] =
"usage: usbipd [options]\n"
+   "\n"
+   "   -4, --ipv4\n"
+   "   Bind to IPv4. Default is both.\n"
+   "\n"
+   "   -6, --ipv6\n"
+   "   Bind to IPv6. Default is both.\n"
+   "\n"
"   -D, --daemon\n"
"   Run as a daemon process.\n"
"\n"
@@ -354,14 +361,15 @@ static void addrinfo_to_text(struct addrinfo *ai, char 
buf[],
snprintf(buf, buf_size, "%s:%s", hbuf, sbuf);
 }
 
-static int listen_all_addrinfo(struct addrinfo *ai_head, int sockfdlist[])
+static int listen_all_addrinfo(struct addrinfo *ai_head, int sockfdlist[],
+int maxsockfd)
 {
struct addrinfo *ai;
int ret, nsockfd = 0;
const size_t ai_buf_size = NI_MAXHOST + NI_MAXSERV + 2;
char ai_buf[ai_buf_size];
 
-   for (ai = ai_head; ai && nsockfd < MAXSOCKFD; ai = ai->ai_next) {
+   for (ai = ai_head; ai && nsockfd < maxsockfd; ai = ai->ai_next) {
int sock;
addrinfo_to_text(ai, ai_buf, ai_buf_size);
dbg("opening %s", ai_buf);
@@ -374,6 +382,9 @@ static int listen_all_addrinfo(struct addrinfo *ai_head, 
int sockfdlist[])
 
usbip_net_set_reuseaddr(sock);
usbip_net_set_nodelay(sock);
+   /* We use seperate sockets for IPv4 and IPv6
+* (see do_standalone_mode()) */
+   usbip_net_set_v6only(sock);
 
if (sock >= FD_SETSIZE) {
err("FD_SETSIZE: %s: sock=%d, max=%d",
@@ -402,11 +413,6 @@ static int listen_all_addrinfo(struct addrinfo *ai_head, 
int sockfdlist[])
sockfdlist[nsockfd++] = sock;
}
 
-   if (nsockfd == 0)
-   return -1;
-
-   dbg("listening on %d address%s", nsockfd, (nsockfd == 1) ? "" : "es");
-
return nsockfd;
 }
 
@@ -473,11 +479,11 @@ static void remove_pid_file()
}
 }
 
-static int do_standalone_mode(int daemonize)
+static int do_standalone_mode(int daemonize, int ipv4, int ipv6)
 {
struct addrinfo *ai_head;
int sockfdlist[MAXSOCKFD];
-   int nsockfd;
+   int nsockfd, family;
int i, terminate;
struct pollfd *fds;
struct timespec timeout;
@@ -501,21 +507,36 @@ static int do_standalone_mode(int daemonize)
set_signal();
write_pid_file();
 
-   ai_head = do_getaddrinfo(NULL, PF_UNSPEC);
+   info("starting " PROGNAME " (%s)", usbip_version_string);
+
+   /*
+* To suppress warnings on systems with bindv6o

[PATCH 10/11] staging: usbip: Separate protocol/program version

2013-09-11 Thread Dominik Paulus
Not all new program versions necessarily introduce
non-backwards-compatible protocol changes. We thus move the definition
of the protocol version from configure.ac to usbip_network.h, where it
logically belongs to.

Signed-off-by: Dominik Paulus 
Signed-off-by: Tobias Polzer 
---
 drivers/staging/usbip/userspace/configure.ac| 1 -
 drivers/staging/usbip/userspace/src/usbip_network.c | 6 +++---
 drivers/staging/usbip/userspace/src/usbip_network.h | 6 ++
 3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/usbip/userspace/configure.ac 
b/drivers/staging/usbip/userspace/configure.ac
index 7bba496..099d24b 100644
--- a/drivers/staging/usbip/userspace/configure.ac
+++ b/drivers/staging/usbip/userspace/configure.ac
@@ -2,7 +2,6 @@ dnl Process this file with autoconf to produce a configure 
script.
 
 AC_PREREQ(2.59)
 AC_INIT([usbip-utils], [1.1.1], [linux-...@vger.kernel.org])
-AC_DEFINE([USBIP_VERSION], [0x0111], [binary-coded decimal version number])
 
 CURRENT=0
 REVISION=1
diff --git a/drivers/staging/usbip/userspace/src/usbip_network.c 
b/drivers/staging/usbip/userspace/src/usbip_network.c
index 61cd8db..f5955c2 100644
--- a/drivers/staging/usbip/userspace/src/usbip_network.c
+++ b/drivers/staging/usbip/userspace/src/usbip_network.c
@@ -153,7 +153,7 @@ int usbip_net_send_op_common(int sockfd, uint32_t code, 
uint32_t status)
 
memset(&op_common, 0, sizeof(op_common));
 
-   op_common.version = USBIP_VERSION;
+   op_common.version = PROTOCOL_VERSION;
op_common.code= code;
op_common.status  = status;
 
@@ -183,9 +183,9 @@ int usbip_net_recv_op_common(int sockfd, uint16_t *code)
 
PACK_OP_COMMON(0, &op_common);
 
-   if (op_common.version != USBIP_VERSION) {
+   if (op_common.version != PROTOCOL_VERSION) {
dbg("version mismatch: %d %d", op_common.version,
-   USBIP_VERSION);
+   PROTOCOL_VERSION);
return -ERR_MISMATCH;
}
 
diff --git a/drivers/staging/usbip/userspace/src/usbip_network.h 
b/drivers/staging/usbip/userspace/src/usbip_network.h
index d3c1b71..6a41fd8 100644
--- a/drivers/staging/usbip/userspace/src/usbip_network.h
+++ b/drivers/staging/usbip/userspace/src/usbip_network.h
@@ -14,6 +14,12 @@
 
 #include 
 
+/*
+ * Protocol version. Incremented only on non-backwards-compatible
+ * changes.
+ */
+#define PROTOCOL_VERSION 0x111
+
 extern int usbip_port;
 extern char *usbip_port_string;
 extern char *usbip_srp_password;
-- 
1.8.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 07/11] staging: usbip: Add proper error reporting

2013-09-11 Thread Dominik Paulus
This patch adds new error codes and features extended error reporting in
op_common packets.

Signed-off-by: Dominik Paulus 
Signed-off-by: Tobias Polzer 
---
 drivers/staging/usbip/userspace/src/usbip_attach.c |  4 +-
 drivers/staging/usbip/userspace/src/usbip_list.c   |  3 +-
 .../staging/usbip/userspace/src/usbip_network.c| 50 --
 .../staging/usbip/userspace/src/usbip_network.h| 17 +++-
 drivers/staging/usbip/userspace/src/usbipd.c   | 29 +++--
 5 files changed, 74 insertions(+), 29 deletions(-)

diff --git a/drivers/staging/usbip/userspace/src/usbip_attach.c 
b/drivers/staging/usbip/userspace/src/usbip_attach.c
index 2363e56..2a3f313 100644
--- a/drivers/staging/usbip/userspace/src/usbip_attach.c
+++ b/drivers/staging/usbip/userspace/src/usbip_attach.c
@@ -147,7 +147,7 @@ static int query_import_device(int sockfd, char *busid)
/* receive a reply */
rc = usbip_net_recv_op_common(sockfd, &code);
if (rc < 0) {
-   err("recv op_common");
+   err("recv op_common: %s", usbip_net_strerror(rc));
return -1;
}
 
@@ -177,7 +177,7 @@ static int attach_device(char *host, char *busid)
 
sockfd = usbip_net_connect(host);
if (sockfd < 0) {
-   err("tcp connect");
+   err("connection attempt failed");
return -1;
}
 
diff --git a/drivers/staging/usbip/userspace/src/usbip_list.c 
b/drivers/staging/usbip/userspace/src/usbip_list.c
index e4fa5b8..ff7acf8 100644
--- a/drivers/staging/usbip/userspace/src/usbip_list.c
+++ b/drivers/staging/usbip/userspace/src/usbip_list.c
@@ -64,7 +64,8 @@ static int get_exported_devices(char *host, int sockfd)
 
rc = usbip_net_recv_op_common(sockfd, &code);
if (rc < 0) {
-   dbg("usbip_net_recv_op_common failed");
+   err("usbip_net_recv_op_common failed: %s",
+   usbip_net_strerror(rc));
return -1;
}
 
diff --git a/drivers/staging/usbip/userspace/src/usbip_network.c 
b/drivers/staging/usbip/userspace/src/usbip_network.c
index eda641f..61cd8db 100644
--- a/drivers/staging/usbip/userspace/src/usbip_network.c
+++ b/drivers/staging/usbip/userspace/src/usbip_network.c
@@ -178,7 +178,7 @@ int usbip_net_recv_op_common(int sockfd, uint16_t *code)
rc = usbip_net_recv(sockfd, &op_common, sizeof(op_common));
if (rc < 0) {
dbg("usbip_net_recv failed: %d", rc);
-   goto err;
+   return -ERR_SYSERR;
}
 
PACK_OP_COMMON(0, &op_common);
@@ -186,30 +186,48 @@ int usbip_net_recv_op_common(int sockfd, uint16_t *code)
if (op_common.version != USBIP_VERSION) {
dbg("version mismatch: %d %d", op_common.version,
USBIP_VERSION);
-   goto err;
+   return -ERR_MISMATCH;
}
 
switch (*code) {
case OP_UNSPEC:
break;
default:
-   if (op_common.code != *code) {
+   /*
+* Only accept expected opcode. Exception: OP_REPLY
+* flag set may be sent as a reply to all requests,
+* if only used for status reporting.
+*/
+   if (op_common.code != *code && op_common.code != OP_REPLY) {
dbg("unexpected pdu %#0x for %#0x", op_common.code,
*code);
-   goto err;
+   return -ERR_UNEXPECTED;
}
}
 
-   if (op_common.status != ST_OK) {
-   dbg("request failed at peer: %d", op_common.status);
-   goto err;
-   }
-
*code = op_common.code;
 
-   return 0;
-err:
-   return -1;
+   return -op_common.status;
+}
+
+const char *usbip_net_strerror(int status)
+{
+   static const char *const errs[] = {
+   /* ERR_OK */ "Success",
+   /* ERR_NA */ "Command failed",
+   /* ERR_MISMATCH */ "Protocol version mismatch",
+   /* ERR_SYSERR */ "System error",
+   /* ERR_UNEXPECTED */ "Unexpected opcode received",
+   /* ERR_AUTHREQ */ "Server requires authentication",
+   /* ERR_PERM */ "Permission denied",
+   /* ERR_NOTFOUND */ "Requested device not found",
+   /* ERR_NOAUTH */ "Server doesn't support authentication"
+   };
+   if (status < 0)
+   status = -status;
+   if (status >= (int) (sizeof(errs) / sizeof(*errs)))
+   return "Invalid";
+   return errs[status];
 }
 
 int usbip_net_set_reuseaddr(int sockfd)
@@ -360,6 +378,7 @@ int usbip_net_connect(char *hostname)
 #ifdef HAVE_GNUTLS
if (usbip_srp_password) {
int rc;
+   uint16_t code = OP_REP_STARTTLS;
 
rc = usbip_net_send_op_common(sockfd, OP_REQ_STARTTLS, 0);
if (rc < 0) {
@@ -3

[PATCH 08/11] staging: usbip: Handle usbip being started as user

2013-09-11 Thread Dominik Paulus
usbip now prints an error message when started as user and requiring
root access. Also, some debug messages are changed to error messages so
the command line utilities now print less confusing (and more verbose)
error messages when not used correctly.

Signed-off-by: Dominik Paulus 
Signed-off-by: Tobias Polzer 
---
 drivers/staging/usbip/userspace/src/usbip_attach.c |  3 +++
 drivers/staging/usbip/userspace/src/usbip_bind.c   | 16 ++--
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/usbip/userspace/src/usbip_attach.c 
b/drivers/staging/usbip/userspace/src/usbip_attach.c
index 2a3f313..651e93a 100644
--- a/drivers/staging/usbip/userspace/src/usbip_attach.c
+++ b/drivers/staging/usbip/userspace/src/usbip_attach.c
@@ -210,6 +210,9 @@ int usbip_attach(int argc, char *argv[])
int opt;
int ret = -1;
 
+   if (geteuid() != 0)
+   err("not running as root?");
+
for (;;) {
opt = getopt_long(argc, argv, "r:b:", opts, NULL);
 
diff --git a/drivers/staging/usbip/userspace/src/usbip_bind.c 
b/drivers/staging/usbip/userspace/src/usbip_bind.c
index d2739fc..ab26b30f 100644
--- a/drivers/staging/usbip/userspace/src/usbip_bind.c
+++ b/drivers/staging/usbip/userspace/src/usbip_bind.c
@@ -158,7 +158,7 @@ static int unbind_other(char *busid)
 
busid_dev = sysfs_open_device(bus_type, busid);
if (!busid_dev) {
-   dbg("sysfs_open_device %s failed: %s", busid, strerror(errno));
+   err("sysfs_open_device %s failed: %s", busid, strerror(errno));
return -1;
}
 
@@ -166,7 +166,7 @@ static int unbind_other(char *busid)
bDevClass  = sysfs_get_device_attr(busid_dev, "bDeviceClass");
bNumIntfs  = sysfs_get_device_attr(busid_dev, "bNumInterfaces");
if (!bConfValue || !bDevClass || !bNumIntfs) {
-   dbg("problem getting device attributes: %s",
+   err("problem getting device attributes: %s",
strerror(errno));
goto err_close_busid_dev;
}
@@ -181,7 +181,7 @@ static int unbind_other(char *busid)
 bConfValue->value, i);
intf_dev = sysfs_open_device(bus_type, intf_busid);
if (!intf_dev) {
-   dbg("could not open interface device: %s",
+   err("could not open interface device: %s",
strerror(errno));
goto err_close_busid_dev;
}
@@ -202,14 +202,14 @@ static int unbind_other(char *busid)
/* unbinding */
intf_drv = sysfs_open_driver(bus_type, intf_dev->driver_name);
if (!intf_drv) {
-   dbg("could not open interface driver on %s: %s",
+   err("could not open interface driver on %s: %s",
intf_dev->name, strerror(errno));
goto err_close_intf_dev;
}
 
unbind_attr = sysfs_get_driver_attr(intf_drv, "unbind");
if (!unbind_attr) {
-   dbg("problem getting interface driver attribute: %s",
+   err("problem getting interface driver attribute: %s",
strerror(errno));
goto err_close_intf_drv;
}
@@ -218,7 +218,8 @@ static int unbind_other(char *busid)
   SYSFS_BUS_ID_SIZE);
if (rc < 0) {
/* NOTE: why keep unbinding other interfaces? */
-   dbg("unbind driver at %s failed", intf_dev->bus_id);
+   err("unbind driver at %s failed: %s", intf_dev->bus_id,
+   strerror(errno));
status = UNBIND_ST_FAILED;
}
 
@@ -287,6 +288,9 @@ int usbip_bind(int argc, char *argv[])
 
allow[0] = 0;
 
+   if (geteuid() != 0)
+   err("not running as root?");
+
for (;;) {
opt = getopt_long(argc, argv, "a:b:", opts, NULL);
 
-- 
1.8.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 06/11] staging: usbip: Add support for ACLs in usbipd

2013-09-11 Thread Dominik Paulus
Interpret the ACLs stored in sysfs in usbipd and reject clients not
matching one of the ACLs.

Signed-off-by: Kurt Kanzenbach 
Signed-off-by: Dominik Paulus 
Signed-off-by: Tobias Polzer 
---
 drivers/staging/usbip/userspace/src/Makefile.am |  2 +-
 drivers/staging/usbip/userspace/src/usbipd.c| 79 +
 2 files changed, 80 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/usbip/userspace/src/Makefile.am 
b/drivers/staging/usbip/userspace/src/Makefile.am
index a113003..5161bae 100644
--- a/drivers/staging/usbip/userspace/src/Makefile.am
+++ b/drivers/staging/usbip/userspace/src/Makefile.am
@@ -9,4 +9,4 @@ usbip_SOURCES := usbip.h utils.h usbip.c utils.c 
usbip_network.c \
 usbip_bind.c usbip_unbind.c
 
 
-usbipd_SOURCES := usbip_network.h usbipd.c usbip_network.c
+usbipd_SOURCES := usbip_network.h usbipd.c usbip_network.c utils.c
diff --git a/drivers/staging/usbip/userspace/src/usbipd.c 
b/drivers/staging/usbip/userspace/src/usbipd.c
index 8db2f27..bc1fd19 100644
--- a/drivers/staging/usbip/userspace/src/usbipd.c
+++ b/drivers/staging/usbip/userspace/src/usbipd.c
@@ -48,6 +48,7 @@
 #include "usbip_host_driver.h"
 #include "usbip_common.h"
 #include "usbip_network.h"
+#include "utils.h"
 
 #undef  PROGNAME
 #define PROGNAME "usbipd"
@@ -169,12 +170,69 @@ static void usbipd_help(void)
printf("%s\n", usbipd_help_string);
 }
 
+/*
+ * Checks whether client IP matches at least one
+ * ACL entry
+ *
+ * Returns:
+ * 1  if matches
+ * 0  if not
+ * -1 on error
+ */
+static int check_allowed(char *acls, int sockfd)
+{
+   int rc, match;
+   struct sockaddr_storage sa;
+   char *acl_cpy, *iter, *saveptr;
+   socklen_t sa_len = sizeof(sa);
+
+   rc = getpeername(sockfd, (struct sockaddr *) &sa, &sa_len);
+   if (rc || sa_len > sizeof(sa)) {
+   err("getpeername failed: %s", strerror(errno));
+   return -1;
+   }
+
+   /*
+* We are going to modify our argument,
+* thus, we need to duplicate it.
+*/
+   acl_cpy = strdup(acls);
+   if (!acl_cpy) {
+   err("strdup(): %s", strerror(errno));
+   return -1;
+   }
+
+   match = 0;
+   iter = strtok_r(acl_cpy, "\n", &saveptr);
+   /*
+* Iterate over ACL entries and check for
+* matching one.
+*/
+   while (iter) {
+   struct subnet net;
+
+   if (parse_cidr(iter, &net) < 0) {
+   dbg("parse_cidr() failed");
+   } else if (in_range(&sa, net)) {
+   match = 1;
+   break;
+   }
+
+   iter = strtok_r(NULL, "\n", &saveptr);
+   }
+
+   free(acl_cpy);
+   return match;
+}
+
 static int recv_request_import(int sockfd)
 {
struct op_import_request req;
struct op_common reply;
struct usbip_exported_device *edev;
struct usbip_usb_device pdu_udev;
+   struct sysfs_attribute *usbip_acl;
+   char ip_attr_path[SYSFS_PATH_MAX];
int found = 0;
int error = 0;
int rc;
@@ -206,6 +264,27 @@ static int recv_request_import(int sockfd)
rc = usbip_host_export_device(edev, sockfd);
if (rc < 0)
error = 1;
+
+   /* check for allowed IPs */
+   snprintf(ip_attr_path, sizeof(ip_attr_path), "%s/%s:%d.%d/%s",
+   edev->udev.path, edev->udev.busid,
+   edev->udev.bConfigurationValue, 0, "usbip_acl");
+
+   usbip_acl = sysfs_open_attribute(ip_attr_path);
+   if (usbip_acl) {
+   rc = sysfs_read_attribute(usbip_acl);
+   if (rc < 0) {
+   err("Unable to open sysfs");
+   error = 1;
+   } else if (check_allowed(usbip_acl->value, sockfd) != 
1) {
+   info("Access denied to device %s",
+   edev->udev.busid);
+   error = 1;
+   }
+   sysfs_close_attribute(usbip_acl);
+   } else {
+   err("failed to get ip list");
+   }
} else {
info("requested device not found: %s", req.busid);
error = 1;
-- 
1.8.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 03/11] staging: usbip: Add kernel support for client ACLs

2013-09-11 Thread Dominik Paulus
This patch adds the possibility to stored ACLs for allowed clients for
each stub device in sysfs. It adds a new sysfs entry called "usbip_acl"
for each stub device, containing a list of CIDR masks of allowed
clients. This file will be used by usbip and usbipd to store the ACL.

Signed-off-by: Kurt Kanzenbach 
Signed-off-by: Dominik Paulus 
Signed-off-by: Tobias Polzer 
---
 drivers/staging/usbip/stub.h |  5 +++
 drivers/staging/usbip/stub_dev.c | 68 +++-
 2 files changed, 72 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/usbip/stub.h b/drivers/staging/usbip/stub.h
index a73e437..cfe75d1 100644
--- a/drivers/staging/usbip/stub.h
+++ b/drivers/staging/usbip/stub.h
@@ -60,6 +60,11 @@ struct stub_device {
struct list_head unlink_free;
 
wait_queue_head_t tx_waitq;
+
+   /* list of allowed IP addrs */
+   char *acls;
+   /* for locking list operations */
+   spinlock_t ip_lock;
 };
 
 /* private data into urb->priv */
diff --git a/drivers/staging/usbip/stub_dev.c b/drivers/staging/usbip/stub_dev.c
index d8957a5..c44d5f2 100644
--- a/drivers/staging/usbip/stub_dev.c
+++ b/drivers/staging/usbip/stub_dev.c
@@ -142,6 +142,62 @@ err:
 }
 static DEVICE_ATTR(usbip_sockfd, S_IWUSR, NULL, store_sockfd);
 
+/*
+ * This function replaces the current ACL list
+ */
+static ssize_t store_acl(struct device *dev, struct device_attribute *attr,
+   const char *buf, size_t count)
+{
+   struct stub_device *sdev = dev_get_drvdata(dev);
+   int retval = 0;
+
+   if (!sdev)
+   return -ENODEV;
+
+   if (count >= PAGE_SIZE)
+   /* Prevent storing oversized ACLs in kernel memory */
+   return -EINVAL;
+
+   /* Store ACL */
+   spin_lock_irq(&sdev->ip_lock);
+   kfree(sdev->acls);
+   sdev->acls = kstrdup(buf, GFP_KERNEL);
+   if (IS_ERR(sdev->acls)) {
+   retval = PTR_ERR(sdev->acls);
+   sdev->acls = NULL;
+   } else {
+   retval = strlen(sdev->acls);
+   }
+   spin_unlock_irq(&sdev->ip_lock);
+
+   return retval;
+}
+
+/*
+ * This functions prints all allowed IP addrs for this dev
+ */
+static ssize_t show_acl(struct device *dev, struct device_attribute *attr,
+  char *buf)
+{
+   struct stub_device *sdev = dev_get_drvdata(dev);
+   int retval = 0;
+
+   if (!sdev)
+   return -ENODEV;
+
+   spin_lock_irq(&sdev->ip_lock);
+   if (sdev->acls == NULL) {
+   retval = 0;
+   } else {
+   strcpy(buf, sdev->acls);
+   retval = strlen(buf);
+   }
+   spin_unlock_irq(&sdev->ip_lock);
+
+   return retval;
+}
+static DEVICE_ATTR(usbip_acl, S_IWUSR | S_IRUGO, show_acl, store_acl);
+
 static int stub_add_files(struct device *dev)
 {
int err = 0;
@@ -157,9 +213,13 @@ static int stub_add_files(struct device *dev)
err = device_create_file(dev, &dev_attr_usbip_debug);
if (err)
goto err_debug;
+   err = device_create_file(dev, &dev_attr_usbip_acl);
+   if (err)
+   goto err_ip;
 
return 0;
-
+err_ip:
+   device_remove_file(dev, &dev_attr_usbip_debug);
 err_debug:
device_remove_file(dev, &dev_attr_usbip_sockfd);
 err_sockfd:
@@ -173,6 +233,7 @@ static void stub_remove_files(struct device *dev)
device_remove_file(dev, &dev_attr_usbip_status);
device_remove_file(dev, &dev_attr_usbip_sockfd);
device_remove_file(dev, &dev_attr_usbip_debug);
+   device_remove_file(dev, &dev_attr_usbip_acl);
 }
 
 static void stub_shutdown_connection(struct usbip_device *ud)
@@ -306,12 +367,14 @@ static struct stub_device *stub_device_alloc(struct 
usb_device *udev,
sdev->ud.status = SDEV_ST_AVAILABLE;
spin_lock_init(&sdev->ud.lock);
sdev->ud.tcp_socket = NULL;
+   sdev->acls = NULL;
 
INIT_LIST_HEAD(&sdev->priv_init);
INIT_LIST_HEAD(&sdev->priv_tx);
INIT_LIST_HEAD(&sdev->priv_free);
INIT_LIST_HEAD(&sdev->unlink_free);
INIT_LIST_HEAD(&sdev->unlink_tx);
+   spin_lock_init(&sdev->ip_lock);
spin_lock_init(&sdev->priv_lock);
 
init_waitqueue_head(&sdev->tx_waitq);
@@ -507,6 +570,9 @@ static void stub_disconnect(struct usb_interface *interface)
usb_put_dev(sdev->udev);
usb_put_intf(interface);
 
+   /* free ACL list */
+   kfree(sdev->acls);
+
/* free sdev */
busid_priv->sdev = NULL;
stub_device_free(sdev);
-- 
1.8.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 09/11] staging: usbip: Improve debug output

2013-09-11 Thread Dominik Paulus
For IPv6, IP:Port is unreadable.

Signed-off-by: Dominik Paulus 
Signed-off-by: Tobias Polzer 
---
 drivers/staging/usbip/userspace/src/usbipd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/usbip/userspace/src/usbipd.c 
b/drivers/staging/usbip/userspace/src/usbipd.c
index ae572c6..6550460 100644
--- a/drivers/staging/usbip/userspace/src/usbipd.c
+++ b/drivers/staging/usbip/userspace/src/usbipd.c
@@ -519,7 +519,7 @@ static int do_accept(int listenfd)
return -1;
}
 #endif
-   info("connection from %s:%s", host, port);
+   info("connection from %s, port %s", host, port);
 
return connfd;
 }
-- 
1.8.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 04/11] staging: usbip: Add CIDR matching helper functions

2013-09-11 Thread Dominik Paulus
This patch adds a few utility functions to match IP addresses against
CIDR masks.

Signed-off-by: Dominik Paulus 
Signed-off-by: Tobias Polzer 
---
 drivers/staging/usbip/userspace/src/utils.c | 84 +
 drivers/staging/usbip/userspace/src/utils.h | 15 ++
 2 files changed, 99 insertions(+)

diff --git a/drivers/staging/usbip/userspace/src/utils.c 
b/drivers/staging/usbip/userspace/src/utils.c
index 2d4966e..df40817 100644
--- a/drivers/staging/usbip/userspace/src/utils.c
+++ b/drivers/staging/usbip/userspace/src/utils.c
@@ -74,3 +74,87 @@ int modify_match_busid(char *busid, int add)
 
return ret;
 }
+
+/*
+ * Parses a string of form "ip/prefix" into a subnet mask to dest.
+ * Returns -1 on error, 0 on success
+ */
+int parse_cidr(const char *src, struct subnet *dest)
+{
+   char *ip, *prefix, *saveptr;
+   char *endptr;
+   struct in6_addr ip6;
+   struct in_addr ip4;
+   int bits;
+   long int tmp;
+   char buf[128]; /* For strtok */
+
+   strncpy(buf, src, sizeof(buf));
+   buf[sizeof(buf)-1] = 0;
+
+   ip = strtok_r(buf, "/", &saveptr);
+   prefix = strtok_r(NULL, "/", &saveptr);
+   if (strtok_r(NULL, "/", &saveptr) || !ip ||
+   strlen(src) > sizeof(buf) - 1)
+   return -1;
+
+   if (inet_pton(AF_INET6, ip, &ip6) == 1) {
+   dest->ai_family = AF_INET6;
+   bits = 128;
+   dest->address.ip6 = ip6;
+   } else if (inet_pton(AF_INET, ip, &ip4) == 1) {
+   dest->ai_family = AF_INET;
+   bits = 32;
+   dest->address.ip4 = ip4;
+   } else {
+   return -1;
+   }
+
+   /*
+* We also accept single IPs without an explicitely
+* specified prefix
+*/
+   if (prefix) {
+   tmp = strtol(prefix, &endptr, 10);
+   if (tmp < 0 || tmp > bits || *endptr != '\0')
+   return -1;
+   dest->prefix = tmp;
+   } else {
+   dest->prefix = bits;
+   }
+
+   return 0;
+}
+
+/*
+ * Checks if addr is in range. Expects addr to be a struct in6_addr* if
+ * ai_family == AF_INET6, else struct in_addr*.
+ * Returns 1 if in range, 0 otherwise.
+ */
+int in_range(struct sockaddr_storage *addr, struct subnet range)
+{
+   if (addr->ss_family != range.ai_family)
+   return 0;
+   if (addr->ss_family == AF_INET6) {
+   int i;
+   struct sockaddr_in6 *in6addr = (struct sockaddr_in6 *) addr;
+   unsigned char *ip = in6addr->sin6_addr.s6_addr;
+   for (i = 0; i < range.prefix; ++i) {
+   int idx = i/8, mask = 1 << (7 - i%8);
+   if ((ip[idx] & mask) != (range.address.ip6.s6_addr[idx]
+ & mask))
+   return 0;
+   }
+   } else {
+   int i;
+   struct sockaddr_in *inaddr = (struct sockaddr_in *) addr;
+   uint32_t ip = ntohl(inaddr->sin_addr.s_addr);
+   uint32_t comp = ntohl(range.address.ip4.s_addr);
+   for (i = 0; i < range.prefix; ++i) {
+   int mask = 1 << (31-i);
+   if ((ip & mask) != (comp & mask))
+   return 0;
+   }
+   }
+   return 1;
+}
diff --git a/drivers/staging/usbip/userspace/src/utils.h 
b/drivers/staging/usbip/userspace/src/utils.h
index 5916fd3..a3704ef 100644
--- a/drivers/staging/usbip/userspace/src/utils.h
+++ b/drivers/staging/usbip/userspace/src/utils.h
@@ -19,7 +19,22 @@
 #ifndef __UTILS_H
 #define __UTILS_H
 
+#include 
+#include 
+#include 
+
+struct subnet {
+   int ai_family;
+   int prefix;
+   union {
+   struct in6_addr ip6;
+   struct in_addr ip4;
+   } address;
+};
+
 int modify_match_busid(char *busid, int add);
+int parse_cidr(const char *src, struct subnet *dest);
+int in_range(struct sockaddr_storage *addr, struct subnet range);
 
 #endif /* __UTILS_H */
 
-- 
1.8.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[no subject]

2013-09-11 Thread Dominik Paulus
Hi,

this patch series includes an updated version of the IPv6 support patch (a call
to freeaddrinfo() was missing) as well as:

 - The client/server authentication support using GnuTLS Tobias already
   announced on the usbip-devel mailing list some time ago[1]

 - Support for restricting the access to devices to specific IP address ranges

 - Improved error reporting and new error codes to be passed over the TCP
   protocol.

We think that the added features justify a version bump to 1.2.0. The
corresponding patch is also included. All protocol changes are
backwards-compatible, thus, we don't increment the protocol version.

Regards,
Tobias Polzer and Dominik Paulus

[1] See <6aeb926e1c4572e79488a91a827333c9.squir...@faumail.uni-erlangen.de>

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 05/11] staging: usbip: Add ACL support to usbip bind

2013-09-11 Thread Dominik Paulus
Add the command line argument -a (--allow) to usbip bind to specify
networks allowed to attach to the device and code to store the ACLs in
sysfs.

Signed-off-by: Kurt Kanzenbach 
Signed-off-by: Dominik Paulus 
Signed-off-by: Tobias Polzer 
---
 drivers/staging/usbip/userspace/doc/usbip.8  |  8 ++-
 drivers/staging/usbip/userspace/src/usbip_bind.c | 74 
 2 files changed, 69 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/usbip/userspace/doc/usbip.8 
b/drivers/staging/usbip/userspace/doc/usbip.8
index b5050ed..b818bde 100644
--- a/drivers/staging/usbip/userspace/doc/usbip.8
+++ b/drivers/staging/usbip/userspace/doc/usbip.8
@@ -62,9 +62,15 @@ Detach an imported USB device.
 .PP
 
 .HP
-\fBbind\fR \-\-busid=<\fIbusid\fR>
+\fBbind\fR \-\-busid=<\fIbusid\fR> [\-\-allow=<\fICIDR mask\fR>...]
 .IP
 Make a device exportable.
+.br
+\-\-allow accepts CIDR masks like 127.0.0.0/8 or fd00::/64
+.br
+Only hosts in (at least) one of the allowed ranges are accepted. If
+\-\-allow is omitted, 0.0.0.0/0 and ::/0 are added to the list. The list can
+be read/written from corresponding \fBusbip_acl\fR file in sysfs after bind.
 .PP
 
 .HP
diff --git a/drivers/staging/usbip/userspace/src/usbip_bind.c 
b/drivers/staging/usbip/userspace/src/usbip_bind.c
index 9ecaf6e..d2739fc 100644
--- a/drivers/staging/usbip/userspace/src/usbip_bind.c
+++ b/drivers/staging/usbip/userspace/src/usbip_bind.c
@@ -37,8 +37,9 @@ enum unbind_status {
 
 static const char usbip_bind_usage_string[] =
"usbip bind \n"
-   "-b, --busid=Bind " USBIP_HOST_DRV_NAME ".ko to device "
-   "on \n";
+   "-b, --busid=Bind " USBIP_HOST_DRV_NAME ".ko to "
+   "device on \n"
+   "-a, --allow=Restrict device access to \n";
 
 void usbip_bind_usage(void)
 {
@@ -46,17 +47,19 @@ void usbip_bind_usage(void)
 }
 
 /* call at unbound state */
-static int bind_usbip(char *busid)
+static int bind_usbip(char *busid, char *allow)
 {
char bus_type[] = "usb";
char attr_name[] = "bind";
char sysfs_mntpath[SYSFS_PATH_MAX];
char bind_attr_path[SYSFS_PATH_MAX];
char intf_busid[SYSFS_BUS_ID_SIZE];
+   char ip_attr_path[SYSFS_PATH_MAX];
struct sysfs_device *busid_dev;
struct sysfs_attribute *bind_attr;
struct sysfs_attribute *bConfValue;
struct sysfs_attribute *bNumIntfs;
+   struct sysfs_attribute *usbip_ip;
int i, failed = 0;
int rc, ret = -1;
 
@@ -101,8 +104,32 @@ static int bind_usbip(char *busid)
dbg("bind driver at %s failed", intf_busid);
failed = 1;
}
+
+   }
+
+   /*
+* store allowed IP ranges
+* specified by `usbip bind -b  --allow `
+*/
+   snprintf(ip_attr_path, sizeof(ip_attr_path),
+   "%s/%s/%s/%s/%s/%s:%.1s.%d/%s",
+   sysfs_mntpath, SYSFS_BUS_NAME, bus_type,
+   SYSFS_DRIVERS_NAME, USBIP_HOST_DRV_NAME, busid,
+   bConfValue->value, 0, "usbip_acl");
+
+   usbip_ip = sysfs_open_attribute(ip_attr_path);
+   if (!usbip_ip) {
+   err("sysfs_open_attribute failed: path=%s",
+   ip_attr_path);
+   goto err_close_busid_dev;
}
 
+   rc = sysfs_write_attribute(usbip_ip, allow, strlen(allow));
+   if (rc)
+   err("sysfs_write_attribute failed");
+
+   sysfs_close_attribute(usbip_ip);
+
if (!failed)
ret = 0;
 
@@ -213,7 +240,7 @@ out:
return status;
 }
 
-static int bind_device(char *busid)
+static int bind_device(char *busid, char *allow)
 {
int rc;
 
@@ -233,7 +260,7 @@ static int bind_device(char *busid)
return -1;
}
 
-   rc = bind_usbip(busid);
+   rc = bind_usbip(busid, allow);
if (rc < 0) {
err("could not bind device to %s", USBIP_HOST_DRV_NAME);
modify_match_busid(busid, 0);
@@ -249,29 +276,52 @@ int usbip_bind(int argc, char *argv[])
 {
static const struct option opts[] = {
{ "busid", required_argument, NULL, 'b' },
+   { "allow", required_argument, NULL, 'a' },
{ NULL,0, NULL,  0  }
};
 
-   int opt;
-   int ret = -1;
+   int opt, rc;
+   char allow[4096];
+   char *device = NULL;
+   struct subnet subnet;
+
+   allow[0] = 0;
 
for (;;) {
-   opt = getopt_long(argc, argv, "b:", opts, NULL);
+   opt = getopt_long(argc, argv, "a:b:", opts, NULL);
 
if (opt == -1)
break;
 
switch (opt) {
+   case 'a':
+   rc = parse_cidr(optarg, &subnet);
+   if (rc < 0) {
+   err("Invalid subnet specified: %s", optarg);
+   go

Re: [PATCH] staging: Disable lustre file system for MIPS, SH, and XTENSA

2013-09-11 Thread Christoph Hellwig
On Wed, Sep 11, 2013 at 10:51:50AM +0800, Peng Tao wrote:
> I'm not fighting against removing the piece of code. But if there is a
> strong reason to keep the functionality, we need to find a way to
> implement it. The convenience of using environment variables is that
> job scheduler can set the environment and other existing applications
> don't have to change. Are there other means to do the same? ioctl and
> upcall both need application change AFAIK.

There is no use case for it, the kernel has no business looking at these
variables.  Given that you think it's not even used I don't even know
why we're having this discussion.

Talking about nasty code, the whole linux-curproc.c is highly
questionable:

 - cfs_curproc_groups_nr:
unused and should be removed

 - cfs_cap_raise/cfs_cap_lower/cfs_cap_raised:
needs to go away, modyules must not change access permissions
on behalf of processes

 - the whole cfs_cap_t handling also needs to go away, passing around
   capabilities is not a concept the kernel supports for a reason

 - current_is_32bit:
Code should just use is_compat_task directly.


I've just taken the time to walk through this one file, but it seems
like most of libcfs is just as bad.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] staging: speakup: kobjects.c: Use correct values when changing voice.

2013-09-11 Thread Christopher Brannon
When a new voice is selected, we set volume and pitch appropriate for
the voice.  We need to use the numeric index corresponding to the
voice when indexing into the volume and pitch tables, rather than
the raw user input that was used to select the voice.
Note that using the raw input can also lead to an invalid memory read
in the case of invalid or malicious user input.

Signed-off-by: Christopher Brannon 
---
 drivers/staging/speakup/kobjects.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/speakup/kobjects.c 
b/drivers/staging/speakup/kobjects.c
index 61a3f7a..f31afa2 100644
--- a/drivers/staging/speakup/kobjects.c
+++ b/drivers/staging/speakup/kobjects.c
@@ -651,7 +651,10 @@ ssize_t spk_var_store(struct kobject *kobj, struct 
kobj_attribute *attr,
* If voice was just changed, we might need to reset our default
* pitch and volume.
*/
-   if (param->var_id == VOICE) {
+   if (param->var_id == VOICE && synth &&
+   (ret == 0 || ret == -ERESTART)) {
+   var_data = param->data;
+   value = var_data->u.n.value;
spk_reset_default_value("pitch", synth->default_pitch,
value);
spk_reset_default_value("vol", synth->default_vol,
-- 
1.8.1.5

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: dgnc: fix potential format string flaw

2013-09-11 Thread Kees Cook
On Wed, Sep 11, 2013 at 2:31 AM, Dan Carpenter  wrote:
> On Tue, Sep 10, 2013 at 10:19:17PM -0700, Kees Cook wrote:
>> In the former case, format characters will get processed by the
>> sprintf logic. In the latter, they are printed as-is. In this specific
>> case, if there was a way to inject strings like "ohai %n" into the
>> msgbuf string, the former would actually attempt to resolve the %n. In
>> the simple case, this could lead to Oopses, and in the unlucky case,
>> it could allow arbitrary memory writing and execution control.
>>
>> http://en.wikipedia.org/wiki/Uncontrolled_format_string
>
> The kernel ignores %n so hopefully it can't actually write to memory.

I wish! This is not the case, though. See FORMAT_TYPE_NRCHARS in
lib/vsprintf.c's vsnprintf().

$ git grep '%n' | wc -l
111

-Kees

-- 
Kees Cook
Chrome OS Security
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: dgnc: fix potential format string flaw

2013-09-11 Thread Joe Perches
On Wed, 2013-09-11 at 11:19 -0700, Kees Cook wrote:
> On Wed, Sep 11, 2013 at 2:31 AM, Dan Carpenter  
> wrote:
> > On Tue, Sep 10, 2013 at 10:19:17PM -0700, Kees Cook wrote:
> >> In the former case, format characters will get processed by the
> >> sprintf logic. In the latter, they are printed as-is. In this specific
> >> case, if there was a way to inject strings like "ohai %n" into the
> >> msgbuf string, the former would actually attempt to resolve the %n. In
> >> the simple case, this could lead to Oopses, and in the unlucky case,
> >> it could allow arbitrary memory writing and execution control.
> >>
> >> http://en.wikipedia.org/wiki/Uncontrolled_format_string
> >
> > The kernel ignores %n so hopefully it can't actually write to memory.
> 
> I wish! This is not the case, though. See FORMAT_TYPE_NRCHARS in
> lib/vsprintf.c's vsnprintf().
> 
> $ git grep '%n' | wc -l
> 111

Umm.

See: lib/vsprintf.c

/**
 * vsnprintf - Format a string and place it in a buffer
[...]
 * %n is ignored

%n does work for vsscanf though.


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: dgnc: fix potential format string flaw

2013-09-11 Thread Kees Cook
On Wed, Sep 11, 2013 at 12:09 PM, Joe Perches  wrote:
> On Wed, 2013-09-11 at 11:19 -0700, Kees Cook wrote:
>> On Wed, Sep 11, 2013 at 2:31 AM, Dan Carpenter  
>> wrote:
>> > On Tue, Sep 10, 2013 at 10:19:17PM -0700, Kees Cook wrote:
>> >> In the former case, format characters will get processed by the
>> >> sprintf logic. In the latter, they are printed as-is. In this specific
>> >> case, if there was a way to inject strings like "ohai %n" into the
>> >> msgbuf string, the former would actually attempt to resolve the %n. In
>> >> the simple case, this could lead to Oopses, and in the unlucky case,
>> >> it could allow arbitrary memory writing and execution control.
>> >>
>> >> http://en.wikipedia.org/wiki/Uncontrolled_format_string
>> >
>> > The kernel ignores %n so hopefully it can't actually write to memory.
>>
>> I wish! This is not the case, though. See FORMAT_TYPE_NRCHARS in
>> lib/vsprintf.c's vsnprintf().
>>
>> $ git grep '%n' | wc -l
>> 111
>
> Umm.
>
> See: lib/vsprintf.c
>
> /**
>  * vsnprintf - Format a string and place it in a buffer
> [...]
>  * %n is ignored
>
> %n does work for vsscanf though.

The comment is a lie:

int len = 0;
printk("len:%d\n", len);
printk("%s%n\n", "Ohai!", &len);
printk("len:%d\n", len);

[0.025930] len:0
[0.026003] Ohai!
[0.026261] len:5

The functionality between scanf and printf was, I think, merged in
2009, if I'm reading the git blame correctly.

-Kees

-- 
Kees Cook
Chrome OS Security
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: dgnc: fix potential format string flaw

2013-09-11 Thread Kees Cook
On Wed, Sep 11, 2013 at 12:22 PM, Dan Carpenter
 wrote:
> On Wed, Sep 11, 2013 at 11:19:11AM -0700, Kees Cook wrote:
>> On Wed, Sep 11, 2013 at 2:31 AM, Dan Carpenter  
>> wrote:
>> > On Tue, Sep 10, 2013 at 10:19:17PM -0700, Kees Cook wrote:
>> >> In the former case, format characters will get processed by the
>> >> sprintf logic. In the latter, they are printed as-is. In this specific
>> >> case, if there was a way to inject strings like "ohai %n" into the
>> >> msgbuf string, the former would actually attempt to resolve the %n. In
>> >> the simple case, this could lead to Oopses, and in the unlucky case,
>> >> it could allow arbitrary memory writing and execution control.
>> >>
>> >> http://en.wikipedia.org/wiki/Uncontrolled_format_string
>> >
>> > The kernel ignores %n so hopefully it can't actually write to memory.
>>
>> I wish! This is not the case, though. See FORMAT_TYPE_NRCHARS in
>> lib/vsprintf.c's vsnprintf().
>>
>> $ git grep '%n' | wc -l
>> 111
>>
>
> Hm...  That's unfortunate.  The comments were shifted around so it says
> it's ignored but it's not.

Outside of scanf, there are very few uses, though:

$ git grep %n | grep print
...
net/phonet/socket.c:seq_printf(seq, "%s%n", "pt  loc  rem
rs st tx_queue rx_queue "
net/phonet/socket.c:seq_printf(seq, "%s%n", "rs   uid inode", &len);
net/phonet/socket.c:seq_printf(seq, "%02X %5u %lu%n",
net/sctp/objcnt.c:  seq_printf(seq, "%s: %d%n", sctp_dbg_objcnt[i].label,

-Kees

-- 
Kees Cook
Chrome OS Security
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: dgnc: fix potential format string flaw

2013-09-11 Thread Dan Carpenter
>  * %n is ignored

Really it should trigger a WARN_ON_ONCE().  There is code like
show_console_dev() which relies on it to work.  If we ignore %n it
causes another bug.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: dgnc: fix potential format string flaw

2013-09-11 Thread Dan Carpenter
On Wed, Sep 11, 2013 at 11:19:11AM -0700, Kees Cook wrote:
> On Wed, Sep 11, 2013 at 2:31 AM, Dan Carpenter  
> wrote:
> > On Tue, Sep 10, 2013 at 10:19:17PM -0700, Kees Cook wrote:
> >> In the former case, format characters will get processed by the
> >> sprintf logic. In the latter, they are printed as-is. In this specific
> >> case, if there was a way to inject strings like "ohai %n" into the
> >> msgbuf string, the former would actually attempt to resolve the %n. In
> >> the simple case, this could lead to Oopses, and in the unlucky case,
> >> it could allow arbitrary memory writing and execution control.
> >>
> >> http://en.wikipedia.org/wiki/Uncontrolled_format_string
> >
> > The kernel ignores %n so hopefully it can't actually write to memory.
> 
> I wish! This is not the case, though. See FORMAT_TYPE_NRCHARS in
> lib/vsprintf.c's vsnprintf().
> 
> $ git grep '%n' | wc -l
> 111
> 

Hm...  That's unfortunate.  The comments were shifted around so it says
it's ignored but it's not.

regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


RE: [PATCH] Drivers: hv: vmbus: fix error return code in vmbus_connect()

2013-09-11 Thread KY Srinivasan


> -Original Message-
> From: Wei Yongjun [mailto:weiyj...@gmail.com]
> Sent: Wednesday, September 11, 2013 4:20 AM
> To: KY Srinivasan; Haiyang Zhang
> Cc: yongjun_...@trendmicro.com.cn; de...@linuxdriverproject.org; linux-
> ker...@vger.kernel.org
> Subject: [PATCH] Drivers: hv: vmbus: fix error return code in vmbus_connect()
> 
> From: Wei Yongjun 
> 
> Fix to return -EINVAL in the version check error handling
> case instead of 0, as done elsewhere in this function.

The return will not be zero in this case. If you look at the function 
vmbus_negotiate_version(), in case the host refuses the version, the
return value will be set to -ECONNREFUSED

Regards,

K. Y
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: dgnc: fix potential format string flaw

2013-09-11 Thread Kees Cook
On Wed, Sep 11, 2013 at 12:45 PM, Joe Perches  wrote:
> On Wed, 2013-09-11 at 12:25 -0700, Kees Cook wrote:
>> On Wed, Sep 11, 2013 at 12:09 PM, Joe Perches  wrote:
>> > On Wed, 2013-09-11 at 11:19 -0700, Kees Cook wrote:
>> >> On Wed, Sep 11, 2013 at 2:31 AM, Dan Carpenter  
>> >> wrote:
>> >> > On Tue, Sep 10, 2013 at 10:19:17PM -0700, Kees Cook wrote:
>> >> >> In the former case, format characters will get processed by the
>> >> >> sprintf logic. In the latter, they are printed as-is. In this specific
>> >> >> case, if there was a way to inject strings like "ohai %n" into the
>> >> >> msgbuf string, the former would actually attempt to resolve the %n. In
>> >> >> the simple case, this could lead to Oopses, and in the unlucky case,
>> >> >> it could allow arbitrary memory writing and execution control.
>> >> >>
>> >> >> http://en.wikipedia.org/wiki/Uncontrolled_format_string
>> >> >
>> >> > The kernel ignores %n so hopefully it can't actually write to memory.
>> >>
>> >> I wish! This is not the case, though. See FORMAT_TYPE_NRCHARS in
>> >> lib/vsprintf.c's vsnprintf().
>> >>
>> >> $ git grep '%n' | wc -l
>> >> 111
>> >
>> > Umm.
>> >
>> > See: lib/vsprintf.c
>> >
>> > /**
>> >  * vsnprintf - Format a string and place it in a buffer
>> > [...]
>> >  * %n is ignored
>> >
>> > %n does work for vsscanf though.
>>
>> The comment is a lie:
>>
>> int len = 0;
>> printk("len:%d\n", len);
>> printk("%s%n\n", "Ohai!", &len);
>> printk("len:%d\n", len);
>>
>> [0.025930] len:0
>> [0.026003] Ohai!
>> [0.026261] len:5
>>
>> The functionality between scanf and printf was, I think, merged in
>> 2009, if I'm reading the git blame correctly.
>
> Yeah.
>
> commit fef20d9c1380f04ba9492d6463148db07b413708
> Author: Frederic Weisbecker 
> Date:   Fri Mar 6 17:21:50 2009 +0100
>
> vsprintf: unify the format decoding layer for its 3 users
>
> Maybe it should be reignored...
>
> There are a few more in net/ though that may be pretty
> easy to change to use the seq_printf return value.

I would love to remove %n. It's not clear to me how to re-split it
from scanf, though. I'd need to study this code a bunch more. Dropping
%n from all its non-scanf uses would be a good first-step, though.

-Kees

>
>
> net/ipv4/fib_trie.c-seq_printf(seq,
> net/ipv4/fib_trie.c- 
> "%s\t%08X\t%08X\t%04X\t%d\t%u\t"
> net/ipv4/fib_trie.c: 
> "%d\t%08X\t%d\t%u\t%u%n",
> net/ipv4/fib_trie.c- fi->fib_dev ? 
> fi->fib_dev->name : "*",
> --
> net/ipv4/fib_trie.c-seq_printf(seq,
> net/ipv4/fib_trie.c- 
> "*\t%08X\t%08X\t%04X\t%d\t%u\t"
> net/ipv4/fib_trie.c: 
> "%d\t%08X\t%d\t%u\t%u%n",
> net/ipv4/fib_trie.c- prefix, 0, flags, 0, 
> 0, 0,
> --
> net/ipv4/ping.c-seq_printf(f, "%5d: %08X:%04X %08X:%04X"
> net/ipv4/ping.c:" %02X %08X:%08X %02X:%08lX %08X %5u %8d %lu 
> %d %pK %d%n",
> net/ipv4/ping.c-bucket, src, srcp, dest, destp, sp->sk_state,
> --
> net/ipv4/tcp_ipv4.c-seq_printf(f, "%4d: %08X:%04X %08X:%04X"
> net/ipv4/tcp_ipv4.c:" %02X %08X:%08X %02X:%08lX %08X %5u %8d %u 
> %d %pK%n",
> net/ipv4/tcp_ipv4.c-i,
> --
> net/ipv4/tcp_ipv4.c-seq_printf(f, "%4d: %08X:%04X %08X:%04X %02X 
> %08X:%08X %02X:%08lX "
> net/ipv4/tcp_ipv4.c:"%08X %5u %8d %lu %d %pK %lu %lu %u 
> %u %d%n",
> net/ipv4/tcp_ipv4.c-i, src, srcp, dest, destp, sk->sk_state,
> --
> net/ipv4/tcp_ipv4.c-seq_printf(f, "%4d: %08X:%04X %08X:%04X"
> net/ipv4/tcp_ipv4.c:" %02X %08X:%08X %02X:%08lX %08X %5d %8d %d 
> %d %pK%n",
> net/ipv4/tcp_ipv4.c-i, src, srcp, dest, destp, tw->tw_substate, 
> 0, 0,
> --
> net/ipv4/udp.c- seq_printf(f, "%5d: %08X:%04X %08X:%04X"
> net/ipv4/udp.c: " %02X %08X:%08X %02X:%08lX %08X %5u %8d %lu %d %pK 
> %d%n",
> net/ipv4/udp.c- bucket, src, srcp, dest, destp, sp->sk_state,
> --
> net/sctp/objcnt.c:  seq_printf(seq, "%s: %d%n", sctp_dbg_objcnt[i].label,
> net/sctp/objcnt.c-  
> atomic_read(sctp_dbg_objcnt[i].counter), &len);
>
>
>



-- 
Kees Cook
Chrome OS Security
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 1/1] staging/speakup/kobjects.c: Code improvement.

2013-09-11 Thread Samuel Thibault
Raphael S.Carvalho, le Mon 02 Sep 2013 19:20:18 -0300, a écrit :
> Well, there is no need to use strcmp since we can make a test of similar 
> semantic by using the var_id field of param.
> I moved the test into the VAR_NUM:VAR_TIME case since VAR_STRING will never 
> be "voice".
> 
> spk_xlate isn't used anymore (in line 628), then there is no difference 
> between using cp and buf in VAR_STRING case.
> Besides, buf is a const char and those changes remove one uneeded line.
> 
> I created the function spk_reset_default_value because it clarifies the code 
> and allows code reusing.
> 
> Signed-off-by: Raphael S.Carvalho 

Acked-by: Samuel Thibault 

> ---
>  drivers/staging/speakup/kobjects.c |   71 
> 
>  1 files changed, 39 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/staging/speakup/kobjects.c 
> b/drivers/staging/speakup/kobjects.c
> index 51bdea3..5c6e77a 100644
> --- a/drivers/staging/speakup/kobjects.c
> +++ b/drivers/staging/speakup/kobjects.c
> @@ -586,6 +586,25 @@ ssize_t spk_var_show(struct kobject *kobj, struct 
> kobj_attribute *attr,
>  EXPORT_SYMBOL_GPL(spk_var_show);
>  
>  /*
> + * Used to reset either default_pitch or default_vol.
> + */
> +static inline void spk_reset_default_value(char *header_name,
> + int *synth_default_value, int idx)
> +{
> + struct st_var_header *param;
> +
> + if (synth && synth_default_value) {
> + param = spk_var_header_by_name(header_name);
> + if (param)  {
> + spk_set_num_var(synth_default_value[idx],
> + param, E_NEW_DEFAULT);
> + spk_set_num_var(0, param, E_DEFAULT);
> + pr_info("%s reset to default value\n", param->name);
> + }
> + }
> +}
> +
> +/*
>   * This function is called when a user echos a value to one of the
>   * variable parameters.
>   */
> @@ -624,56 +643,44 @@ ssize_t spk_var_store(struct kobject *kobj, struct 
> kobj_attribute *attr,
>   if (ret == -ERANGE) {
>   var_data = param->data;
>   pr_warn("value for %s out of range, expect %d to %d\n",
> - attr->attr.name,
> + param->name,
>   var_data->u.n.low, var_data->u.n.high);
>   }
> +
> +/*
> + * If voice was just changed, we might need to reset our default
> + * pitch and volume.
> + */
> + if (param->var_id == VOICE) {
> + spk_reset_default_value("pitch", synth->default_pitch,
> + value);
> + spk_reset_default_value("vol", synth->default_vol,
> + value);
> + }
>   break;
>   case VAR_STRING:
> - len = strlen(buf);
> - if ((len >= 1) && (buf[len - 1] == '\n'))
> + len = strlen(cp);
> + if ((len >= 1) && (cp[len - 1] == '\n'))
>   --len;
> - if ((len >= 2) && (buf[0] == '"') && (buf[len - 1] == '"')) {
> - ++buf;
> + if ((len >= 2) && (cp[0] == '"') && (cp[len - 1] == '"')) {
> + ++cp;
>   len -= 2;
>   }
> - cp = (char *) buf;
>   cp[len] = '\0';
> - ret = spk_set_string_var(buf, param, len);
> + ret = spk_set_string_var(cp, param, len);
>   if (ret == -E2BIG)
>   pr_warn("value too long for %s\n",
> - attr->attr.name);
> + param->name);
>   break;
>   default:
>   pr_warn("%s unknown type %d\n",
>   param->name, (int)param->var_type);
>   break;
> - }
> - /*
> -  * If voice was just changed, we might need to reset our default
> -  * pitch and volume.
> -  */
> - if (strcmp(attr->attr.name, "voice") == 0) {
> - if (synth && synth->default_pitch) {
> - param = spk_var_header_by_name("pitch");
> - if (param)  {
> - spk_set_num_var(synth->default_pitch[value],
> - param, E_NEW_DEFAULT);
> - spk_set_num_var(0, param, E_DEFAULT);
> - }
> - }
> - if (synth && synth->default_vol) {
> - param = spk_var_header_by_name("vol");
> - if (param)  {
> - spk_set_num_var(synth->default_vol[value],
> - param, E_NEW_DEFAULT);
> - spk_set_num_var(0, param, E_DEFAULT);
> - }
> - }
> - }
> + }
>   spin_u

Re: [PATCH] staging: Disable lustre file system for MIPS, SH, and XTENSA

2013-09-11 Thread Dilger, Andreas
On 2013/09/10 8:25 PM, "Peng Tao"  wrote:

>On Wed, Sep 11, 2013 at 9:44 AM, Christoph Hellwig 
>wrote:
>> On Wed, Sep 11, 2013 at 01:14:11AM +0800, Peng Tao wrote:
>>> The problem is access_process_vm() is not exported since certain
>>> version of kernel including the latest. According to Christoph in the
>>> other mail, access_process_vm() is also a core mm function that is not
>>> supposed to be exported. Then what kind of change shall we make in
>>> order to keep current functionality?
>>
>> You should remove the higher level functionality, kernel modules are
>> not supposed to look at userspace environment variables.
>>
>OK. I've looked at the specific case that Lustre uses
>access_process_vm() to get the jobid environment variable and package
>it into the RPC requests to server. However, it turns out that in the
>latest Lustre server code, the jobid in a request is not used
>anywhere. So it looks like we can just get rid of it.
>
>Andreas, could you please confirm this? Is the jobid an obsolete
>parameter that can be abandoned? Or is there plan to use it somehow in
>the future?

The jobid code is relatively new and in use, I'm not sure why you think
it is not in use.  It is actually a feature that a bunch of users
requested.

The jobid feature allows tracking IO request stats for parallel user
processes
running on possibly thousands of different client nodes onto the servers.
This is easy to do with a single node and a single process via PID/PPID
and blktrace or equivalent, but otherwise impossible in a parallel
processing
environment where there may be users running hundreds of different jobs.
The PID/PPID is not consistent across client nodes, and the server threads
will randomly handle requests from all users.

By all means, I'd prefer to just use access_process_vm() directly, instead
of making a copy in the Lustre code.  Not being able to access the process
environment seems a bit ridiculous - the kernel stores and manages this for
the process, and it isn't even doing anything nasty like accessing the
environment from a different process, just its own environment variables.

Cheers, Andreas
-- 
Andreas Dilger

Lustre Software Architect
Intel High Performance Data Division


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: speakup: kobjects.c: Use correct values when changing voice.

2013-09-11 Thread Samuel Thibault
Christopher Brannon, le Tue 10 Sep 2013 18:21:18 -0700, a écrit :
> When a new voice is selected, we set volume and pitch appropriate for
> the voice.  We need to use the numeric index corresponding to the
> voice when indexing into the volume and pitch tables, rather than
> the raw user input that was used to select the voice.
> Note that using the raw input can also lead to an invalid memory read
> in the case of invalid or malicious user input.
> 
> Signed-off-by: Christopher Brannon 

Acked-by: Samuel Thibault 

> ---
>  drivers/staging/speakup/kobjects.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/speakup/kobjects.c 
> b/drivers/staging/speakup/kobjects.c
> index 51bdea3..55aeeb2 100644
> --- a/drivers/staging/speakup/kobjects.c
> +++ b/drivers/staging/speakup/kobjects.c
> @@ -652,7 +652,10 @@ ssize_t spk_var_store(struct kobject *kobj, struct 
> kobj_attribute *attr,
>* If voice was just changed, we might need to reset our default
>* pitch and volume.
>*/
> - if (strcmp(attr->attr.name, "voice") == 0) {
> + if (strcmp(attr->attr.name, "voice") == 0 &&
> + (ret == 0 || ret == -ERESTART)) {
> + var_data = param->data;
> + value = var_data->u.n.value;
>   if (synth && synth->default_pitch) {
>   param = spk_var_header_by_name("pitch");
>   if (param)  {
> -- 
> 1.8.1.5
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] Staging: lustre: fix formatting mistake in Kconfig

2013-09-11 Thread Jon Bernard
Signed-off-by: Jon Bernard 
---
 drivers/staging/lustre/lustre/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/Kconfig 
b/drivers/staging/lustre/lustre/Kconfig
index 4e898e4..f949728 100644
--- a/drivers/staging/lustre/lustre/Kconfig
+++ b/drivers/staging/lustre/lustre/Kconfig
@@ -16,7 +16,7 @@ config LUSTRE_FS
  this file system support as a module, choose M here: the module will
  be called lustre.
 
- To mount Lustre file systems , you also need to install the user space
+ To mount Lustre file systems, you also need to install the user space
  mount.lustre and other user space commands which can be found in the
  lustre-client package, available from
  http://downloads.whamcloud.com/public/lustre/
-- 
1.8.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: speakup: kobjects.c: Use correct values when changing voice.

2013-09-11 Thread Samuel Thibault
Christopher Brannon, le Wed 11 Sep 2013 10:05:27 -0700, a écrit :
> When a new voice is selected, we set volume and pitch appropriate for
> the voice.  We need to use the numeric index corresponding to the
> voice when indexing into the volume and pitch tables, rather than
> the raw user input that was used to select the voice.
> Note that using the raw input can also lead to an invalid memory read
> in the case of invalid or malicious user input.
> 
> Signed-off-by: Christopher Brannon 

Acked-by: Samuel Thibault 

> ---
>  drivers/staging/speakup/kobjects.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/speakup/kobjects.c 
> b/drivers/staging/speakup/kobjects.c
> index 61a3f7a..f31afa2 100644
> --- a/drivers/staging/speakup/kobjects.c
> +++ b/drivers/staging/speakup/kobjects.c
> @@ -651,7 +651,10 @@ ssize_t spk_var_store(struct kobject *kobj, struct 
> kobj_attribute *attr,
>   * If voice was just changed, we might need to reset our default
>   * pitch and volume.
>   */
> - if (param->var_id == VOICE) {
> + if (param->var_id == VOICE && synth &&
> + (ret == 0 || ret == -ERESTART)) {
> + var_data = param->data;
> + value = var_data->u.n.value;
>   spk_reset_default_value("pitch", synth->default_pitch,
>   value);
>   spk_reset_default_value("vol", synth->default_vol,
> -- 
> 1.8.1.5
> 

-- 
Samuel
Hi ! I'm a .signature virus ! Copy me into your ~/.signature, please !
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: Disable lustre file system for MIPS, SH, and XTENSA

2013-09-11 Thread Dilger, Andreas
On 2013/09/11 10:29 AM, "Christoph Hellwig"  wrote:
>Talking about nasty code, the whole linux-curproc.c is highly
>questionable:
>
> - cfs_curproc_groups_nr:
>   unused and should be removed

This is already removed in the upstream Lustre code, it just hasn't made
it into the kernel yet.

>- cfs_cap_raise/cfs_cap_lower/cfs_cap_raised:
>   needs to go away, modyules must not change access permissions
>   on behalf of processes

These are only used on the server so that threads running as user UID/GID
can write to recovery log files.  There is likely a different way that
this could be done, it has probably been this way from years ago when we
used the VFS to do file IO on the server and it was doing file permission
checking again.

> - the whole cfs_cap_t handling also needs to go away, passing around
>   capabilities is not a concept the kernel supports for a reason
>
> - current_is_32bit:
>   Code should just use is_compat_task directly.

This was already removed in the upstream Lustre code.

>I've just taken the time to walk through this one file, but it seems
>like most of libcfs is just as bad.

Sure, and that's why Lustre is in drivers/staging and not fs/, and I don't
make any claims to the contrary.  We are slowly cleaning out these macros
(added when we wanted to get Lustre working on MacOS and WinNT), but it
will take some time.  XFS lived with an IRIX shim layer for years, and
still
has a whole vnode abstraction layer that nobody seems to be complaining
about, so I don't think it is unreasonable for us to take some time to
clean
up Lustre.

Cheers, Andreas
-- 
Andreas Dilger

Lustre Software Architect
Intel High Performance Data Division


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: lustre: clean up format string usages

2013-09-11 Thread Dilger, Andreas
On 2013/09/10 10:37 PM, "Kees Cook"  wrote:
>This fixes up the usage of snprintf, strncpy, and format strings in the
>call to kthread_run to avoid ever accidentally allowing a format string
>into the thread name.
>
>Signed-off-by: Kees Cook 

No objection, though I don't think there was any security risk here.
All of these thread names originate in the kernel.

Cheers, Andreas

>---
> .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c |2 +-
> .../staging/lustre/lnet/klnds/socklnd/socklnd_cb.c |2 +-
> drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c|4 ++--
> drivers/staging/lustre/lustre/libcfs/workitem.c|2 +-
> drivers/staging/lustre/lustre/ptlrpc/pinger.c  |4 ++--
> drivers/staging/lustre/lustre/ptlrpc/ptlrpcd.c |8 
> drivers/staging/lustre/lustre/ptlrpc/service.c |6 +++---
> 7 files changed, 14 insertions(+), 14 deletions(-)
>
>diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
>b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
>index 086ca3d..26b49a2 100644
>--- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
>+++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
>@@ -1802,7 +1802,7 @@ kiblnd_recv (lnet_ni_t *ni, void *private,
>lnet_msg_t *lntmsg, int delayed,
> int
> kiblnd_thread_start(int (*fn)(void *arg), void *arg, char *name)
> {
>-  struct task_struct *task = kthread_run(fn, arg, name);
>+  struct task_struct *task = kthread_run(fn, arg, "%s", name);
> 
>   if (IS_ERR(task))
>   return PTR_ERR(task);
>diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
>b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
>index 2c581b7..68a4f52 100644
>--- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
>+++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c
>@@ -1005,7 +1005,7 @@ ksocknal_send(lnet_ni_t *ni, void *private,
>lnet_msg_t *lntmsg)
> int
> ksocknal_thread_start(int (*fn)(void *arg), void *arg, char *name)
> {
>-  struct task_struct *task = kthread_run(fn, arg, name);
>+  struct task_struct *task = kthread_run(fn, arg, "%s", name);
> 
>   if (IS_ERR(task))
>   return PTR_ERR(task);
>diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c
>b/drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c
>index 3916bda..a100a0b 100644
>--- a/drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c
>+++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lockd.c
>@@ -800,9 +800,9 @@ static int ldlm_bl_thread_start(struct ldlm_bl_pool
>*blp)
> 
>   init_completion(&bltd.bltd_comp);
>   bltd.bltd_num = atomic_read(&blp->blp_num_threads);
>-  snprintf(bltd.bltd_name, sizeof(bltd.bltd_name) - 1,
>+  snprintf(bltd.bltd_name, sizeof(bltd.bltd_name),
>   "ldlm_bl_%02d", bltd.bltd_num);
>-  task = kthread_run(ldlm_bl_thread_main, &bltd, bltd.bltd_name);
>+  task = kthread_run(ldlm_bl_thread_main, &bltd, "%s", bltd.bltd_name);
>   if (IS_ERR(task)) {
>   CERROR("cannot start LDLM thread ldlm_bl_%02d: rc %ld\n",
>  atomic_read(&blp->blp_num_threads), PTR_ERR(task));
>diff --git a/drivers/staging/lustre/lustre/libcfs/workitem.c
>b/drivers/staging/lustre/lustre/libcfs/workitem.c
>index 462172d..1a55c81 100644
>--- a/drivers/staging/lustre/lustre/libcfs/workitem.c
>+++ b/drivers/staging/lustre/lustre/libcfs/workitem.c
>@@ -397,7 +397,7 @@ cfs_wi_sched_create(char *name, struct cfs_cpt_table
>*cptab,
>sched->ws_name, sched->ws_nthreads);
>   }
> 
>-  task = kthread_run(cfs_wi_scheduler, sched, name);
>+  task = kthread_run(cfs_wi_scheduler, sched, "%s", name);
>   if (!IS_ERR(task)) {
>   nthrs--;
>   continue;
>diff --git a/drivers/staging/lustre/lustre/ptlrpc/pinger.c
>b/drivers/staging/lustre/lustre/ptlrpc/pinger.c
>index 227a0ae..5dec771 100644
>--- a/drivers/staging/lustre/lustre/ptlrpc/pinger.c
>+++ b/drivers/staging/lustre/lustre/ptlrpc/pinger.c
>@@ -383,8 +383,8 @@ int ptlrpc_start_pinger(void)
> 
>   /* CLONE_VM and CLONE_FILES just avoid a needless copy, because we
>* just drop the VM and FILES in cfs_daemonize_ctxt() right away. */
>-  rc = PTR_ERR(kthread_run(ptlrpc_pinger_main,
>-   &pinger_thread, pinger_thread.t_name));
>+  rc = PTR_ERR(kthread_run(ptlrpc_pinger_main, &pinger_thread,
>+   "%s", pinger_thread.t_name));
>   if (IS_ERR_VALUE(rc)) {
>   CERROR("cannot start thread: %d\n", rc);
>   return rc;
>diff --git a/drivers/staging/lustre/lustre/ptlrpc/ptlrpcd.c
>b/drivers/staging/lustre/lustre/ptlrpc/ptlrpcd.c
>index fbdeff6..89c9be9 100644
>--- a/drivers/staging/lustre/lustre/ptlrpc/ptlrpcd.c
>+++ b/drivers/staging/lustre/lustre/ptlrpc/ptlrpcd.c
>@@ -615,7 +615,7 @@ int ptlrpcd_start(int index, int max, const char
>*name, struct ptlrpcd_ctl *pc)
>   init_co

Re: [PATCH] staging: dgnc: fix potential format string flaw

2013-09-11 Thread Joe Perches
On Wed, 2013-09-11 at 12:25 -0700, Kees Cook wrote:
> On Wed, Sep 11, 2013 at 12:09 PM, Joe Perches  wrote:
> > On Wed, 2013-09-11 at 11:19 -0700, Kees Cook wrote:
> >> On Wed, Sep 11, 2013 at 2:31 AM, Dan Carpenter  
> >> wrote:
> >> > On Tue, Sep 10, 2013 at 10:19:17PM -0700, Kees Cook wrote:
> >> >> In the former case, format characters will get processed by the
> >> >> sprintf logic. In the latter, they are printed as-is. In this specific
> >> >> case, if there was a way to inject strings like "ohai %n" into the
> >> >> msgbuf string, the former would actually attempt to resolve the %n. In
> >> >> the simple case, this could lead to Oopses, and in the unlucky case,
> >> >> it could allow arbitrary memory writing and execution control.
> >> >>
> >> >> http://en.wikipedia.org/wiki/Uncontrolled_format_string
> >> >
> >> > The kernel ignores %n so hopefully it can't actually write to memory.
> >>
> >> I wish! This is not the case, though. See FORMAT_TYPE_NRCHARS in
> >> lib/vsprintf.c's vsnprintf().
> >>
> >> $ git grep '%n' | wc -l
> >> 111
> >
> > Umm.
> >
> > See: lib/vsprintf.c
> >
> > /**
> >  * vsnprintf - Format a string and place it in a buffer
> > [...]
> >  * %n is ignored
> >
> > %n does work for vsscanf though.
> 
> The comment is a lie:
> 
> int len = 0;
> printk("len:%d\n", len);
> printk("%s%n\n", "Ohai!", &len);
> printk("len:%d\n", len);
> 
> [0.025930] len:0
> [0.026003] Ohai!
> [0.026261] len:5
> 
> The functionality between scanf and printf was, I think, merged in
> 2009, if I'm reading the git blame correctly.

Yeah.

commit fef20d9c1380f04ba9492d6463148db07b413708
Author: Frederic Weisbecker 
Date:   Fri Mar 6 17:21:50 2009 +0100

vsprintf: unify the format decoding layer for its 3 users

Maybe it should be reignored...

There are a few more in net/ though that may be pretty
easy to change to use the seq_printf return value.


net/ipv4/fib_trie.c-seq_printf(seq,
net/ipv4/fib_trie.c- 
"%s\t%08X\t%08X\t%04X\t%d\t%u\t"
net/ipv4/fib_trie.c: 
"%d\t%08X\t%d\t%u\t%u%n",
net/ipv4/fib_trie.c- fi->fib_dev ? 
fi->fib_dev->name : "*",
--
net/ipv4/fib_trie.c-seq_printf(seq,
net/ipv4/fib_trie.c- 
"*\t%08X\t%08X\t%04X\t%d\t%u\t"
net/ipv4/fib_trie.c: 
"%d\t%08X\t%d\t%u\t%u%n",
net/ipv4/fib_trie.c- prefix, 0, flags, 0, 
0, 0,
--
net/ipv4/ping.c-seq_printf(f, "%5d: %08X:%04X %08X:%04X"
net/ipv4/ping.c:" %02X %08X:%08X %02X:%08lX %08X %5u %8d %lu %d 
%pK %d%n",
net/ipv4/ping.c-bucket, src, srcp, dest, destp, sp->sk_state,
--
net/ipv4/tcp_ipv4.c-seq_printf(f, "%4d: %08X:%04X %08X:%04X"
net/ipv4/tcp_ipv4.c:" %02X %08X:%08X %02X:%08lX %08X %5u %8d %u %d 
%pK%n",
net/ipv4/tcp_ipv4.c-i,
--
net/ipv4/tcp_ipv4.c-seq_printf(f, "%4d: %08X:%04X %08X:%04X %02X %08X:%08X 
%02X:%08lX "
net/ipv4/tcp_ipv4.c:"%08X %5u %8d %lu %d %pK %lu %lu %u %u 
%d%n",
net/ipv4/tcp_ipv4.c-i, src, srcp, dest, destp, sk->sk_state,
--
net/ipv4/tcp_ipv4.c-seq_printf(f, "%4d: %08X:%04X %08X:%04X"
net/ipv4/tcp_ipv4.c:" %02X %08X:%08X %02X:%08lX %08X %5d %8d %d %d 
%pK%n",
net/ipv4/tcp_ipv4.c-i, src, srcp, dest, destp, tw->tw_substate, 0, 
0,
--
net/ipv4/udp.c- seq_printf(f, "%5d: %08X:%04X %08X:%04X"
net/ipv4/udp.c: " %02X %08X:%08X %02X:%08lX %08X %5u %8d %lu %d %pK 
%d%n",
net/ipv4/udp.c- bucket, src, srcp, dest, destp, sp->sk_state,
--
net/sctp/objcnt.c:  seq_printf(seq, "%s: %d%n", sctp_dbg_objcnt[i].label,
net/sctp/objcnt.c-  
atomic_read(sctp_dbg_objcnt[i].counter), &len);



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [RFC PATCH] vsnprintf: Remove use of %n and convert existing uses

2013-09-11 Thread Kees Cook
On Wed, Sep 11, 2013 at 4:22 PM, Joe Perches  wrote:
> Using vsnprintf or its derivatives with %n can have security
> vulnerability implications.
>
> Prior to commit fef20d9c1380
> ("vsprintf: unify the format decoding layer for its 3 users"),
> any use of %n was ignored.
>
> Reintroduce this feature and convert the existing uses of %n
> to use the return length from vsnprintf or its derivatives.
>
> Signed-off-by: Joe Perches 
> Acked-by: KOSAKI Motohiro  (proc bits)
> cc: Kees Cook 
> cc: Frederic Weisbecker 

Yes, please. It might also be worth updating
Documentation/printk-formats.txt to mention that %n has intentionally
removed and will be ignored.

Reviewed-by: Kees Cook 

-Kees

>
> ---
>
> Not particularly well tested...
>
>  fs/proc/consoles.c   |  2 +-
>  fs/proc/nommu.c  | 20 ++---
>  fs/proc/task_mmu.c   | 18 +--
>  fs/proc/task_nommu.c | 20 ++---
>  lib/vsprintf.c   | 21 ++---
>  net/ipv4/fib_trie.c  | 30 ---
>  net/ipv4/ping.c  | 19 ++--
>  net/ipv4/tcp_ipv4.c  | 84 
> +---
>  net/ipv4/udp.c   | 19 ++--
>  net/phonet/socket.c  | 32 ++--
>  net/sctp/objcnt.c|  5 ++--
>  11 files changed, 132 insertions(+), 138 deletions(-)
>
> diff --git a/fs/proc/consoles.c b/fs/proc/consoles.c
> index b701eaa..42f2bb7 100644
> --- a/fs/proc/consoles.c
> +++ b/fs/proc/consoles.c
> @@ -47,7 +47,7 @@ static int show_console_dev(struct seq_file *m, void *v)
> con_flags[a].name : ' ';
> flags[a] = 0;
>
> -   seq_printf(m, "%s%d%n", con->name, con->index, &len);
> +   len = seq_printf(m, "%s%d", con->name, con->index);
> len = 21 - len;
> if (len < 1)
> len = 1;
> diff --git a/fs/proc/nommu.c b/fs/proc/nommu.c
> index ccfd99b..91cfd19 100644
> --- a/fs/proc/nommu.c
> +++ b/fs/proc/nommu.c
> @@ -50,16 +50,16 @@ static int nommu_region_show(struct seq_file *m, struct 
> vm_region *region)
> ino = inode->i_ino;
> }
>
> -   seq_printf(m,
> -  "%08lx-%08lx %c%c%c%c %08llx %02x:%02x %lu %n",
> -  region->vm_start,
> -  region->vm_end,
> -  flags & VM_READ ? 'r' : '-',
> -  flags & VM_WRITE ? 'w' : '-',
> -  flags & VM_EXEC ? 'x' : '-',
> -  flags & VM_MAYSHARE ? flags & VM_SHARED ? 'S' : 's' : 'p',
> -  ((loff_t)region->vm_pgoff) << PAGE_SHIFT,
> -  MAJOR(dev), MINOR(dev), ino, &len);
> +   len = seq_printf(m, "%08lx-%08lx %c%c%c%c %08llx %02x:%02x %lu ",
> +region->vm_start,
> +region->vm_end,
> +flags & VM_READ ? 'r' : '-',
> +flags & VM_WRITE ? 'w' : '-',
> +flags & VM_EXEC ? 'x' : '-',
> +flags & VM_MAYSHARE ?
> +flags & VM_SHARED ? 'S' : 's' : 'p',
> +((loff_t)region->vm_pgoff) << PAGE_SHIFT,
> +MAJOR(dev), MINOR(dev), ino);
>
> if (file) {
> len = 25 + sizeof(void *) * 6 - len;
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index 107d026..f84ee9f 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -286,15 +286,15 @@ show_map_vma(struct seq_file *m, struct vm_area_struct 
> *vma, int is_pid)
> if (stack_guard_page_end(vma, end))
> end -= PAGE_SIZE;
>
> -   seq_printf(m, "%08lx-%08lx %c%c%c%c %08llx %02x:%02x %lu %n",
> -   start,
> -   end,
> -   flags & VM_READ ? 'r' : '-',
> -   flags & VM_WRITE ? 'w' : '-',
> -   flags & VM_EXEC ? 'x' : '-',
> -   flags & VM_MAYSHARE ? 's' : 'p',
> -   pgoff,
> -   MAJOR(dev), MINOR(dev), ino, &len);
> +   len = seq_printf(m, "%08lx-%08lx %c%c%c%c %08llx %02x:%02x %lu ",
> +start,
> +end,
> +flags & VM_READ ? 'r' : '-',
> +flags & VM_WRITE ? 'w' : '-',
> +flags & VM_EXEC ? 'x' : '-',
> +flags & VM_MAYSHARE ? 's' : 'p',
> +pgoff,
> +MAJOR(dev), MINOR(dev), ino);
>
> /*
>  * Print the dentry name for named mappings, and a
> diff --git a/fs/proc/task_nommu.c b/fs/proc/task_nommu.c
> index 56123a6..1d7bbe5 100644
> --- a/fs/proc/task_nommu.c
> +++ b/fs/proc/task_nommu.c
> @@ -155,16 +155,16 @@ static int nommu_vma_show(struct seq_file *m, struct 
> vm_area_struct *vma,
> pgoff = (loff_t)vma->vm_pgoff << PAGE_SHIFT;
> }
>
> -   seq_printf(m,
> -  "%08lx-%08lx %c%c%c%c %08llx %02x:%02x %lu %n",
> -

Re: BUG: scheduling while atomic 3.10.7 in ZRAM Swap

2013-09-11 Thread Mitch Harder
On Tue, Aug 20, 2013 at 9:51 AM, Mitch Harder
 wrote:
> On Sun, Aug 18, 2013 at 11:44 PM, Minchan Kim  wrote:
>> Hello,
>>
>> On Mon, Aug 19, 2013 at 12:13:02PM +0800, Michael wang wrote:
>>> Hi, Mitch
>>>
>>> On 08/17/2013 10:01 PM, Mitch Harder wrote:
>>> > I'm encountering a BUG while using a ZRAM Swap device.
>>> >
>>> > The call trace seems to involve the changes recently added to 3.10.6
>>> > by the patch:
>>> > zram: use zram->lock to protect zram_free_page() in swap free notify path
>>> >
>>> > The hardware is a x86 single CPU AMD Athlon XP system with 1GB RAM.
>>> >
>>> > I'm implementing a 352MB ZRAM swap device, and also have 1GB swap
>>> > space on the hard disk.
>>>
>>> IMHO, it was caused by that swap_entry_free() was invoked with page
>>> spin-locked, thus zram_slot_free_notify() should not use rw-lock which
>>> may goto sleep.
>>>
>>> CC folks related.
>>
>> Thanks for Ccing me, Michael,
>>
>> Mitch, It's known problem and it should be fixed by [1] in recent linux-next.
>>
>> [1] a0c516cbfc, zram: don't grab mutex in zram_slot_free_noity
>>
>> Thanks for the report!
>>
>
> Thanks.
>
> If I apply the zram patches from linux-next, the problem seems to be resolved.

Is it planned to send the patch: "zram: don't grab mutex in
zram_slot_free_noity"  to stable?

I noticed that 3.10.11 still doesn't have this patch.

Right now, I'm manually applying 4 zram patches to my 3.10.11 kernel
(although I had to rework them to apply cleanly):

zram: Add auto loading of module if user opens /dev/zram.
zram: prevent data loss in error cases of function zram_bvec_write()
zram: fix invalid memory access
zram: don't grab mutex in zram_slot_free_noity

I knew I'd get errors if I didn't rework the "zram: Add auto loading
of module if user opens /dev/zram" patch to apply to 3.10.  The other
three patches seemed to address important issues also, based on their
git commit description.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[RFC PATCH] vsnprintf: Remove use of %n and convert existing uses

2013-09-11 Thread Joe Perches
Using vsnprintf or its derivatives with %n can have security
vulnerability implications.

Prior to commit fef20d9c1380
("vsprintf: unify the format decoding layer for its 3 users"),
any use of %n was ignored.

Reintroduce this feature and convert the existing uses of %n
to use the return length from vsnprintf or its derivatives.

Signed-off-by: Joe Perches 
Acked-by: KOSAKI Motohiro  (proc bits)
cc: Kees Cook 
cc: Frederic Weisbecker 

---

Not particularly well tested...

 fs/proc/consoles.c   |  2 +-
 fs/proc/nommu.c  | 20 ++---
 fs/proc/task_mmu.c   | 18 +--
 fs/proc/task_nommu.c | 20 ++---
 lib/vsprintf.c   | 21 ++---
 net/ipv4/fib_trie.c  | 30 ---
 net/ipv4/ping.c  | 19 ++--
 net/ipv4/tcp_ipv4.c  | 84 +---
 net/ipv4/udp.c   | 19 ++--
 net/phonet/socket.c  | 32 ++--
 net/sctp/objcnt.c|  5 ++--
 11 files changed, 132 insertions(+), 138 deletions(-)

diff --git a/fs/proc/consoles.c b/fs/proc/consoles.c
index b701eaa..42f2bb7 100644
--- a/fs/proc/consoles.c
+++ b/fs/proc/consoles.c
@@ -47,7 +47,7 @@ static int show_console_dev(struct seq_file *m, void *v)
con_flags[a].name : ' ';
flags[a] = 0;
 
-   seq_printf(m, "%s%d%n", con->name, con->index, &len);
+   len = seq_printf(m, "%s%d", con->name, con->index);
len = 21 - len;
if (len < 1)
len = 1;
diff --git a/fs/proc/nommu.c b/fs/proc/nommu.c
index ccfd99b..91cfd19 100644
--- a/fs/proc/nommu.c
+++ b/fs/proc/nommu.c
@@ -50,16 +50,16 @@ static int nommu_region_show(struct seq_file *m, struct 
vm_region *region)
ino = inode->i_ino;
}
 
-   seq_printf(m,
-  "%08lx-%08lx %c%c%c%c %08llx %02x:%02x %lu %n",
-  region->vm_start,
-  region->vm_end,
-  flags & VM_READ ? 'r' : '-',
-  flags & VM_WRITE ? 'w' : '-',
-  flags & VM_EXEC ? 'x' : '-',
-  flags & VM_MAYSHARE ? flags & VM_SHARED ? 'S' : 's' : 'p',
-  ((loff_t)region->vm_pgoff) << PAGE_SHIFT,
-  MAJOR(dev), MINOR(dev), ino, &len);
+   len = seq_printf(m, "%08lx-%08lx %c%c%c%c %08llx %02x:%02x %lu ",
+region->vm_start,
+region->vm_end,
+flags & VM_READ ? 'r' : '-',
+flags & VM_WRITE ? 'w' : '-',
+flags & VM_EXEC ? 'x' : '-',
+flags & VM_MAYSHARE ?
+flags & VM_SHARED ? 'S' : 's' : 'p',
+((loff_t)region->vm_pgoff) << PAGE_SHIFT,
+MAJOR(dev), MINOR(dev), ino);
 
if (file) {
len = 25 + sizeof(void *) * 6 - len;
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 107d026..f84ee9f 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -286,15 +286,15 @@ show_map_vma(struct seq_file *m, struct vm_area_struct 
*vma, int is_pid)
if (stack_guard_page_end(vma, end))
end -= PAGE_SIZE;
 
-   seq_printf(m, "%08lx-%08lx %c%c%c%c %08llx %02x:%02x %lu %n",
-   start,
-   end,
-   flags & VM_READ ? 'r' : '-',
-   flags & VM_WRITE ? 'w' : '-',
-   flags & VM_EXEC ? 'x' : '-',
-   flags & VM_MAYSHARE ? 's' : 'p',
-   pgoff,
-   MAJOR(dev), MINOR(dev), ino, &len);
+   len = seq_printf(m, "%08lx-%08lx %c%c%c%c %08llx %02x:%02x %lu ",
+start,
+end,
+flags & VM_READ ? 'r' : '-',
+flags & VM_WRITE ? 'w' : '-',
+flags & VM_EXEC ? 'x' : '-',
+flags & VM_MAYSHARE ? 's' : 'p',
+pgoff,
+MAJOR(dev), MINOR(dev), ino);
 
/*
 * Print the dentry name for named mappings, and a
diff --git a/fs/proc/task_nommu.c b/fs/proc/task_nommu.c
index 56123a6..1d7bbe5 100644
--- a/fs/proc/task_nommu.c
+++ b/fs/proc/task_nommu.c
@@ -155,16 +155,16 @@ static int nommu_vma_show(struct seq_file *m, struct 
vm_area_struct *vma,
pgoff = (loff_t)vma->vm_pgoff << PAGE_SHIFT;
}
 
-   seq_printf(m,
-  "%08lx-%08lx %c%c%c%c %08llx %02x:%02x %lu %n",
-  vma->vm_start,
-  vma->vm_end,
-  flags & VM_READ ? 'r' : '-',
-  flags & VM_WRITE ? 'w' : '-',
-  flags & VM_EXEC ? 'x' : '-',
-  flags & VM_MAYSHARE ? flags & VM_SHARED ? 'S' : 's' : 'p',
-  pgoff,
-  MAJOR(dev), MINOR(dev), ino, &len);
+   len = seq_printf(m, "%08lx-%08lx %c%c%c%c %08llx %02x:%02x %lu ",
+ 

Re: [PATCH] Drivers: hv: vmbus: fix error return code in vmbus_connect()

2013-09-11 Thread Wei Yongjun
On 09/12/2013 04:03 AM, KY Srinivasan wrote:
>
>> -Original Message-
>> From: Wei Yongjun [mailto:weiyj...@gmail.com]
>> Sent: Wednesday, September 11, 2013 4:20 AM
>> To: KY Srinivasan; Haiyang Zhang
>> Cc: yongjun_...@trendmicro.com.cn; de...@linuxdriverproject.org; linux-
>> ker...@vger.kernel.org
>> Subject: [PATCH] Drivers: hv: vmbus: fix error return code in vmbus_connect()
>>
>> From: Wei Yongjun 
>>
>> Fix to return -EINVAL in the version check error handling
>> case instead of 0, as done elsewhere in this function.
> The return will not be zero in this case. If you look at the function 
> vmbus_negotiate_version(), in case the host refuses the version, the
> return value will be set to -ECONNREFUSED

look at the code:

 196 do {
 197 ret = vmbus_negotiate_version(msginfo, version);
 198 if (ret)
 199 goto cleanup;
 200 
 201 if (vmbus_connection.conn_state == CONNECTED)
 202 break;
 203 
 204 version = vmbus_get_next_version(version);
 205 } while (version != VERSION_INVAL);
 206 
 207 if (version == VERSION_INVAL)
 208 goto cleanup;

if function vmbus_negotiate_version() return error, the code
will goto cleanup. 

If 'version == VERSION_INVAL' is true, I think we tried all
of the VERSION_WS2008/VERSION_WIN7/VERSION_WIN8, but still
can not get the connection.

 



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] Staging: fwserial: wrap a line that exceeds 80 characters

2013-09-11 Thread Peter Hurley

On 09/10/2013 06:00 PM, Jon Bernard wrote:

This is a patch to fwserial.c that wraps a line which previously exceeded the 80
character limit warning found by checkpatch.pl.  This driver is now warning and
error free, according to checkpatch.pl

Signed-off-by: Jon Bernard 


Ok with me, thanks.

Reviewed-by: Peter Hurley 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [RFC PATCH] vsnprintf: Remove use of %n and convert existing uses

2013-09-11 Thread Tetsuo Handa
Joe Perches wrote:
> - seq_printf(m, "%s%d%n", con->name, con->index, &len);
> + len = seq_printf(m, "%s%d", con->name, con->index);

Isn't len always 0 or -1 ?

int seq_vprintf(struct seq_file *m, const char *f, va_list args)
{
int len;

if (m->count < m->size) {
len = vsnprintf(m->buf + m->count, m->size - m->count, f, args);
if (m->count + len < m->size) {
m->count += len;
return 0;
}
}
seq_set_overflow(m);
return -1;
}
EXPORT_SYMBOL(seq_vprintf);

int seq_printf(struct seq_file *m, const char *f, ...)
{
int ret;
va_list args;

va_start(args, f);
ret = seq_vprintf(m, f, args);
va_end(args);

return ret;
}
EXPORT_SYMBOL(seq_printf);
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


RE: [PATCH] Drivers: hv: vmbus: fix error return code in vmbus_connect()

2013-09-11 Thread KY Srinivasan


> -Original Message-
> From: Wei Yongjun [mailto:weiyj...@gmail.com]
> Sent: Wednesday, September 11, 2013 5:26 PM
> To: KY Srinivasan
> Cc: Haiyang Zhang; yongjun_...@trendmicro.com.cn;
> de...@linuxdriverproject.org; linux-ker...@vger.kernel.org
> Subject: Re: [PATCH] Drivers: hv: vmbus: fix error return code in
> vmbus_connect()
> 
> On 09/12/2013 04:03 AM, KY Srinivasan wrote:
> >
> >> -Original Message-
> >> From: Wei Yongjun [mailto:weiyj...@gmail.com]
> >> Sent: Wednesday, September 11, 2013 4:20 AM
> >> To: KY Srinivasan; Haiyang Zhang
> >> Cc: yongjun_...@trendmicro.com.cn; de...@linuxdriverproject.org; linux-
> >> ker...@vger.kernel.org
> >> Subject: [PATCH] Drivers: hv: vmbus: fix error return code in 
> >> vmbus_connect()
> >>
> >> From: Wei Yongjun 
> >>
> >> Fix to return -EINVAL in the version check error handling
> >> case instead of 0, as done elsewhere in this function.
> > The return will not be zero in this case. If you look at the function
> > vmbus_negotiate_version(), in case the host refuses the version, the
> > return value will be set to -ECONNREFUSED
> 
> look at the code:
> 
>  196 do {
>  197 ret = vmbus_negotiate_version(msginfo, version);
>  198 if (ret)
>  199 goto cleanup;
>  200
>  201 if (vmbus_connection.conn_state == CONNECTED)
>  202 break;
>  203
>  204 version = vmbus_get_next_version(version);
>  205 } while (version != VERSION_INVAL);
>  206
>  207 if (version == VERSION_INVAL)
>  208 goto cleanup;
> 
> if function vmbus_negotiate_version() return error, the code
> will goto cleanup.
> 
> If 'version == VERSION_INVAL' is true, I think we tried all
> of the VERSION_WS2008/VERSION_WIN7/VERSION_WIN8, but still
> can not get the connection.

When you try a given version, vmbus_negotiate_version() returns an error code
if the host does not support that version. Hence, in this code you will not be 
able to
run on anything prior to ws2012. I have already sent in a patch to fix that - 
if we timeout
only then we jump to cleanup. In any event, if we tried all versions and none 
was acceptable
to the host, we would return the value from vmbus_negotiate_version().

K. Y
> 
> 
> 
> 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [RFC PATCH] vsnprintf: Remove use of %n and convert existing uses

2013-09-11 Thread Joe Perches
On Wed, 2013-09-11 at 16:29 -0700, Kees Cook wrote:
> On Wed, Sep 11, 2013 at 4:22 PM, Joe Perches  wrote:
> > Using vsnprintf or its derivatives with %n can have security
> > vulnerability implications.
> >
> > Prior to commit fef20d9c1380
> > ("vsprintf: unify the format decoding layer for its 3 users"),
> > any use of %n was ignored.
> >
> > Reintroduce this feature and convert the existing uses of %n
> > to use the return length from vsnprintf or its derivatives.
> >
> > Signed-off-by: Joe Perches 
> > Acked-by: KOSAKI Motohiro  (proc bits)
> > cc: Kees Cook 
> > cc: Frederic Weisbecker 
> 
> Yes, please. It might also be worth updating
> Documentation/printk-formats.txt to mention that %n has intentionally
> removed and will be ignored.

Fine with me if you want to update that file.

It doesn't currently try to be a complete man page
for vsnprintf though.

vsprintf.c does have kernel-doc documentation and
that already does show that %n is ignored.


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [RFC PATCH] vsnprintf: Remove use of %n and convert existing uses

2013-09-11 Thread Al Viro
On Wed, Sep 11, 2013 at 05:04:17PM -0700, Joe Perches wrote:
> On Thu, 2013-09-12 at 08:40 +0900, Tetsuo Handa wrote:
> > Joe Perches wrote:
> > > - seq_printf(m, "%s%d%n", con->name, con->index, &len);
> > > + len = seq_printf(m, "%s%d", con->name, con->index);
> > 
> > Isn't len always 0 or -1 ?
> 
> Right.  Well you're no fun...
> 
> These uses would seem broken anyway because the
> seq_printf isn't itself tested for correctness.
> 
> Hmm.
> 
> Also, there's a large amount of code that appears
> to do calculations with pos or len like:
> 
>   pos += seq_printf(handle, fmt. ...)

... and most of that code proceeds to ignore pos completely.
Note that ->show() is *NOT* supposed to return the number of
characters it has/would like to have produced.  Just return
0 and be done with that; overflows are dealt with just fine.
The large amount, BTW, is below 100 lines, AFAICS, in rather
few files.

> There are very few that seem to use it correctly
> like netfilter.

> Suggestions?

Just bury the cargo-culting crap.  All those += seq_printf() should
be simply calling it.  The *only* reason to look at the return
value is "if we'd already overflown the buffer, I'd rather skipped
the costly generation of the rest of the record".  In that case
seq_printf() returning -1 means "skip it, nothing else will fit and
caller will be repeating with bigger buffer anyway".
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [RFC PATCH] vsnprintf: Remove use of %n and convert existing uses

2013-09-11 Thread Joe Perches
On Thu, 2013-09-12 at 01:19 +0100, Al Viro wrote:
> On Wed, Sep 11, 2013 at 05:04:17PM -0700, Joe Perches wrote:
> > On Thu, 2013-09-12 at 08:40 +0900, Tetsuo Handa wrote:
> > > Joe Perches wrote:
> > > > -   seq_printf(m, "%s%d%n", con->name, con->index, &len);
> > > > +   len = seq_printf(m, "%s%d", con->name, con->index);
> > > 
> > > Isn't len always 0 or -1 ?
> > 
> > Right.  Well you're no fun...
> > 
> > These uses would seem broken anyway because the
> > seq_printf isn't itself tested for correctness.
> > 
> > Hmm.
> > 
> > Also, there's a large amount of code that appears
> > to do calculations with pos or len like:
> > 
> > pos += seq_printf(handle, fmt. ...)
> 
> ... and most of that code proceeds to ignore pos completely.
> Note that ->show() is *NOT* supposed to return the number of
> characters it has/would like to have produced.  Just return
> 0 and be done with that; overflows are dealt with just fine.
> The large amount, BTW, is below 100 lines, AFAICS, in rather
> few files.

Unfortunately, when you count the uses of
return seq_printf(...)
it's rather higher and all the callers need
to be chased down too.

$ grep -rP --include=*.[ch] "^[ \t]*(\S[ 
\t\S]*=|return[\s\(]*)\s*\bseq_[v]?printf\b" * | wc -l
320
$ grep -rPl --include=*.[ch] "^[ \t]*(\S[ 
\t\S]*=|return[\s\(]*)\s*\bseq_[v]?printf\b" *|wc -l
81

> Just bury the cargo-culting crap.  All those += seq_printf() should
> be simply calling it.

Most likely.

> The *only* reason to look at the return
> value is "if we'd already overflown the buffer, I'd rather skipped
> the costly generation of the rest of the record".  In that case
> seq_printf() returning -1 means "skip it, nothing else will fit and
> caller will be repeating with bigger buffer anyway".

Perhaps changing the seq_vprintf return from 0 to len
and testing for -1 would work.

Still would need to change a few lines in netfilter
and probably a few other places.


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel