Re: [PATCH 3/5] Staging/iio/adc/touchscreen/MXS: add i.MX23 support to the LRADC driver
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
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.
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
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
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
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
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
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
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
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
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
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
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
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
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()
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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]
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
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
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.
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
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
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
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
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
> * %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
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()
> -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
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.
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
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.
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
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.
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
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
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
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
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
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
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()
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
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
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()
> -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
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
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
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