Re: [PATCH] f2fs: introduce f2fs_kmem_cache_alloc to hide the unfailed kmem cache allocation
On 10/22/2013 11:49 AM, Gao feng wrote: > On 10/21/2013 03:24 PM, Gu Zheng wrote: >> +static inline void *f2fs_kmem_cache_alloc(struct kmem_cache *cachep, >> +gfp_t flags) >> +{ >> +void *entry = kmem_cache_alloc(cachep, flags); >> +retry: > > retry after kmem_cache_alloc? Oh~, stupid mistake, thanks for your reminder.:) > >> +if (!entry) { >> +cond_resched(); >> +goto retry; >> +} >> + >> +return entry; >> +} > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] f2fs: introduce f2fs_kmem_cache_alloc to hide the unfailed kmem cache allocation
On 10/22/2013 01:16 PM, Haicheng Li wrote: > On Tue, Oct 22, 2013 at 11:49:58AM +0800, Gao feng wrote: >> On 10/21/2013 03:24 PM, Gu Zheng wrote: >>> +static inline void *f2fs_kmem_cache_alloc(struct kmem_cache *cachep, >>> + gfp_t flags) >>> +{ >>> + void *entry = kmem_cache_alloc(cachep, flags); >>> +retry: >> >> retry after kmem_cache_alloc? > > Good catch. > > Sorry for the carelessness in my previous review. > Besides this, I also found another issue as below: > >> On Mon, Oct 21, 2013 at 03:24:55PM +0800, Gu Zheng wrote: >>> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c >>> index ef80f79..fe3cf8e 100644 >>> --- a/fs/f2fs/node.c >>> +++ b/fs/f2fs/node.c >>> @@ -1308,11 +1308,7 @@ static int add_free_nid(struct f2fs_nm_info *nm_i, >>> nid_t nid, bool build) >>> if (allocated) >>> return 0; >>> retry: > -retry? Can be removed here, this tag still used by front goto jumping. But it seems that we need to use another suitable name rather than "retry". Regards, Gu > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] drivers/misc: add rawio framework driver
On Mon, Oct 21, 2013 at 05:03:20PM -0700, Bin Gao wrote: > Rawio provides a framework to read/write registers from a bus, including > pci, i2c, I/O device(memory mapped), etc. based on debug fs. > Rawio bus drivers implement the read/write operation on a specific bus > on top of the rawio framework driver. > They are designed to help device driver and kernel debugging on > embedded systems. Oh, one more technical thing, you totally fail to document the user/kernel api you have just created. You are parsing userspace data in ways that I really don't understand at all... greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/4] drivers/misc: add rawio framework and drivers
On Mon, Oct 21, 2013 at 05:03:17PM -0700, Bin Gao wrote: > To read/write registers from a device is very important on embedded system, > especially SoC systems. Physically there could be different types of devices > based on bus tyes, e.g. PCI devices, I2C (slave)devices, I/O devices(memory > mapped), inter-processor devices, etc. Typically there are userland > tools from > PC Linux to access device registers, but on some embedded system initrd and > rootfs come with a minimal busybox and most useful userland tools are not > available. To add these tools back to rootfs is not convenient either. > What's more, on some systems with runtime pm enabled, reading/writing > registers > from a device which is in low power state will cause problems. For these > reasons, to have some tools/interfaces directly from kernel space via debug > fs seems to be easy, cheap and convenient. So, just because userspace is "hard" you want to add stuff to the kernel instead. Sorry, but for over the past decade, we have been doing just the opposite, if things can be done in userspace, then they should be done there. So for us to go in the opposite direction, like these patches show, would be a major change. > These patchsets are designed to achieve above goals to ease > device driver and kernel debugging on embedded systems. > > Rawio provides a framework to read/write registers from a bus, including > pci, i2c, I/O device(memory mapped), etc. based on debug fs. > Rawio bus drivers implement the read/write operation on a specific bus > on top of the rawio framework driver. > Currently only three bus drivers are available: pci, iomem and i2c. You can already do this today for PCI with the UIO framework, right? Why duplicate that functionality here with another userapce API that we will then have to maintain for the next 40+ years? > But it's extremely easy to add more drivers on top of the framework > if needed. > > drivers/misc/Kconfig | 1 + > drivers/misc/Makefile| 1 + > drivers/misc/rawio/Kconfig | 59 + > drivers/misc/rawio/Makefile | 4 + > drivers/misc/rawio/rawio.c | 514 > +++ All of your patches are line-wrapped and totally fail to apply, so even if we wanted to take this type of changes, I couldn't :( Have you run these proposed changes by any of the Intel kernel developers? What did they say to them? If not, why haven't you, isn't that a resource you should be using for things like this? greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 01/23] crypto: provide single place for hash algo information
On Mon, Oct 21, 2013 at 06:42:46PM -0400, Mimi Zohar wrote: > From: Dmitry Kasatkin > > This patch provides a single place for information about hash algorithms, > such as hash sizes and kernel driver names, which will be used by IMA > and the public key code. > > Changelog: > - Fix sparse and checkpatch warnings > - Move hash algo enums to uapi for userspace signing functions. > > Signed-off-by: Dmitry Kasatkin > Signed-off-by: Mimi Zohar Are you adding a user-space interface for this? If so please use the existing crypto user-space API. Thanks, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] f2fs: introduce f2fs_kmem_cache_alloc to hide the unfailed kmem cache allocation
On Tue, Oct 22, 2013 at 11:49:58AM +0800, Gao feng wrote: > On 10/21/2013 03:24 PM, Gu Zheng wrote: > > +static inline void *f2fs_kmem_cache_alloc(struct kmem_cache *cachep, > > + gfp_t flags) > > +{ > > + void *entry = kmem_cache_alloc(cachep, flags); > > +retry: > > retry after kmem_cache_alloc? Good catch. Sorry for the carelessness in my previous review. Besides this, I also found another issue as below: > On Mon, Oct 21, 2013 at 03:24:55PM +0800, Gu Zheng wrote: > > diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c > > index ef80f79..fe3cf8e 100644 > > --- a/fs/f2fs/node.c > > +++ b/fs/f2fs/node.c > > @@ -1308,11 +1308,7 @@ static int add_free_nid(struct f2fs_nm_info *nm_i, > > nid_t nid, bool build) > > if (allocated) > > return 0; > > retry: -retry? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] f2fs: introduce f2fs_kmem_cache_alloc to hide the unfailed kmem cache allocation
On Tue, Oct 22, 2013 at 10:45:48AM +0800, Haicheng Li wrote: > Looks neat. > > Reviewed-by: Haicheng Li pls. ignore this mail. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -next] staging/iio/adc: MXS_LRADC depends on INPUT
Dear Randy Dunlap, > From: Randy Dunlap > > mxs-lradc.c uses many input_() functions so it should > depend on INPUT to fix build errors. > > drivers/built-in.o: In function `mxs_lradc_ts_unregister': > drivers/staging/iio/adc/mxs-lradc.c:905: undefined reference to > `input_unregister_device' drivers/staging/iio/adc/mxs-lradc.c:905: > undefined reference to `input_unregister_device' drivers/built-in.o: In > function `input_report_abs': > include/linux/input.h:399: undefined reference to `input_event' > include/linux/input.h:399: undefined reference to `input_event' > include/linux/input.h:399: undefined reference to `input_event' > drivers/built-in.o: In function `input_report_key': > include/linux/input.h:389: undefined reference to `input_event' > drivers/built-in.o: In function `input_sync': > include/linux/input.h:414: undefined reference to `input_event' > drivers/built-in.o:include/linux/input.h:389: more undefined references to > `input_event' follow > > Signed-off-by: Randy Dunlap Acked-by: Marek Vasut Thanks! Best regards, Marek Vasut -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH -next] staging/iio/adc: MXS_LRADC depends on INPUT
From: Randy Dunlap mxs-lradc.c uses many input_() functions so it should depend on INPUT to fix build errors. drivers/built-in.o: In function `mxs_lradc_ts_unregister': drivers/staging/iio/adc/mxs-lradc.c:905: undefined reference to `input_unregister_device' drivers/staging/iio/adc/mxs-lradc.c:905: undefined reference to `input_unregister_device' drivers/built-in.o: In function `input_report_abs': include/linux/input.h:399: undefined reference to `input_event' include/linux/input.h:399: undefined reference to `input_event' include/linux/input.h:399: undefined reference to `input_event' drivers/built-in.o: In function `input_report_key': include/linux/input.h:389: undefined reference to `input_event' drivers/built-in.o: In function `input_sync': include/linux/input.h:414: undefined reference to `input_event' drivers/built-in.o:include/linux/input.h:389: more undefined references to `input_event' follow Signed-off-by: Randy Dunlap --- drivers/staging/iio/adc/Kconfig |1 + 1 file changed, 1 insertion(+) --- next-2013-1021.orig/drivers/staging/iio/adc/Kconfig +++ next-2013-1021/drivers/staging/iio/adc/Kconfig @@ -114,6 +114,7 @@ config LPC32XX_ADC config MXS_LRADC tristate "Freescale i.MX23/i.MX28 LRADC" depends on ARCH_MXS || COMPILE_TEST + depends on INPUT select STMP_DEVICE select IIO_BUFFER select IIO_TRIGGERED_BUFFER -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH net-next] net: remove function sk_reset_txq()
What sk_reset_txq() does is just calls function sk_tx_queue_reset(), and sk_reset_txq() is used only in sock.h, by dst_negative_advice(). Let dst_negative_advice() calls sk_tx_queue_reset() directly so we can remove unneeded sk_reset_txq(). Signed-off-by: ZHAO Gang --- include/net/sock.h | 4 +--- net/core/sock.c| 6 -- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/include/net/sock.h b/include/net/sock.h index 1d37a80..db66682 100644 --- a/include/net/sock.h +++ b/include/net/sock.h @@ -1752,8 +1752,6 @@ sk_dst_get(struct sock *sk) return dst; } -extern void sk_reset_txq(struct sock *sk); - static inline void dst_negative_advice(struct sock *sk) { struct dst_entry *ndst, *dst = __sk_dst_get(sk); @@ -1763,7 +1761,7 @@ static inline void dst_negative_advice(struct sock *sk) if (ndst != dst) { rcu_assign_pointer(sk->sk_dst_cache, ndst); - sk_reset_txq(sk); + sk_tx_queue_clear(sk); } } } diff --git a/net/core/sock.c b/net/core/sock.c index 5b6beba..3f1545f 100644 --- a/net/core/sock.c +++ b/net/core/sock.c @@ -475,12 +475,6 @@ discard_and_relse: } EXPORT_SYMBOL(sk_receive_skb); -void sk_reset_txq(struct sock *sk) -{ - sk_tx_queue_clear(sk); -} -EXPORT_SYMBOL(sk_reset_txq); - struct dst_entry *__sk_dst_check(struct sock *sk, u32 cookie) { struct dst_entry *dst = __sk_dst_get(sk); -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] lib/genalloc: add a helper function for DMA buffer allocation
When using pool space for DMA buffer, there might be duplicated calling of gen_pool_alloc() and gen_pool_virt_to_phys() in each implementation. Thus it's better to add a simple helper function, a compatible one to the common dma_alloc_coherent(), to save some code. Signed-off-by: Nicolin Chen --- include/linux/genalloc.h | 2 ++ lib/genalloc.c | 28 2 files changed, 30 insertions(+) diff --git a/include/linux/genalloc.h b/include/linux/genalloc.h index f8d41cb..1eda33d 100644 --- a/include/linux/genalloc.h +++ b/include/linux/genalloc.h @@ -94,6 +94,8 @@ static inline int gen_pool_add(struct gen_pool *pool, unsigned long addr, } extern void gen_pool_destroy(struct gen_pool *); extern unsigned long gen_pool_alloc(struct gen_pool *, size_t); +extern void *gen_pool_dma_alloc(struct gen_pool *pool, size_t size, + dma_addr_t *dma); extern void gen_pool_free(struct gen_pool *, unsigned long, size_t); extern void gen_pool_for_each_chunk(struct gen_pool *, void (*)(struct gen_pool *, struct gen_pool_chunk *, void *), void *); diff --git a/lib/genalloc.c b/lib/genalloc.c index 26cf20b..dda3116 100644 --- a/lib/genalloc.c +++ b/lib/genalloc.c @@ -313,6 +313,34 @@ retry: EXPORT_SYMBOL(gen_pool_alloc); /** + * gen_pool_dma_alloc - allocate special memory from the pool for DMA usage + * @pool: pool to allocate from + * @size: number of bytes to allocate from the pool + * @dma: dma-view physical address + * + * Allocate the requested number of bytes from the specified pool. + * Uses the pool allocation function (with first-fit algorithm by default). + * Can not be used in NMI handler on architectures without + * NMI-safe cmpxchg implementation. + */ +void *gen_pool_dma_alloc(struct gen_pool *pool, size_t size, dma_addr_t *dma) +{ + unsigned long vaddr; + + if (!pool) + return NULL; + + vaddr = gen_pool_alloc(pool, size); + if (!vaddr) + return NULL; + + *dma = gen_pool_virt_to_phys(pool, vaddr); + + return (void *)vaddr; +} +EXPORT_SYMBOL(gen_pool_dma_alloc); + +/** * gen_pool_free - free allocated special memory back to the pool * @pool: pool to free to * @addr: starting address of memory to free back to pool -- 1.8.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] f2fs: introduce f2fs_kmem_cache_alloc to hide the unfailed kmem cache allocation
On 10/21/2013 03:24 PM, Gu Zheng wrote: > +static inline void *f2fs_kmem_cache_alloc(struct kmem_cache *cachep, > + gfp_t flags) > +{ > + void *entry = kmem_cache_alloc(cachep, flags); > +retry: retry after kmem_cache_alloc? > + if (!entry) { > + cond_resched(); > + goto retry; > + } > + > + return entry; > +} -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 02/13] uprobes: allow ignoring of probe hits
On 10/19/13 13:02, Oleg Nesterov wrote: On 10/15, David Long wrote: @@ -1732,9 +1732,6 @@ static void handle_swbp(struct pt_regs *regs) return; } - /* change it in advance for ->handler() and restart */ - instruction_pointer_set(regs, bp_vaddr); - Well, this looks obviously wrong. This SET_IP() has the comment ;) Note also that with this breaks __skip_sstep() on x86. Oleg. Yes, and there's a missing weak stub function in there too. It was a surprise to me that declaring an external as weak means that it quietly ignores the fact there is no definition for it at link time, and makes it zero. I think there may be some similar land mines elsewhere in the kernel, unrelated to these changes or uprobes in general. I have an updated version to go out with the v3 patches. It is working with v3.12-rc6 on x86 and ARM, to the extent I'm able to test it. -dl -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Re: [tpmdd-devel] TPM patches for 3.12
On Tue, Oct 22, 2013 at 01:50:39AM +0200, Peter Huewe wrote: > Jason, can you please review and test my changes - > I'd probably send the pull request out tomorrow then. > (giving me also the chance to think again whether I should add myself to > MAINTAINERS or not ;) Looks Ok, there are no major diffs against what I had. Do you need to add your sign off lines on these patches? tpm: Rename tpm.c to tpm-interface.c tpm: Merge the tpm-bios module with tpm.o Thanks, Jason -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Revert "bridge: only expire the mdb entry when query is received"
On Sat, Oct 19, 2013 at 3:58 PM, Linus Lüssing wrote: > While this commit was a good attempt to fix issues occuring when no > multicast querier is present, this commit still has two more issues: > > 1) There are cases where mdb entries do not expire even if there is a > querier present. The bridge will unnecessarily continue flooding > multicast packets on the according ports. > > 2) Never removing an mdb entry could be exploited for a Denial of > Service by an attacker on the local link, slowly, but steadily eating up > all memory. > I raised the first issue too when I sent the patch, but Herbert said it is not a problem. So, I will leave this to Herbert to decide. For me, your patch makes sense. Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [BUG] spi/spi-atmel: DMA rx buffer corruption with SPI_IOC_MESSAGE
> -Original Message- > From: Richard Genoud [mailto:richard.gen...@gmail.com] > Sent: 2013年10月21日 16:26 > To: Yang, Wenyou > Cc: linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; Ferre, Nicolas; > Mark Brown > Subject: Re: [BUG] spi/spi-atmel: DMA rx buffer corruption with > SPI_IOC_MESSAGE > > 2013/10/21 Yang, Wenyou : > > Hi Richard, > > > >> -Original Message- > >> From: Richard Genoud [mailto:richard.gen...@gmail.com] > >> Sent: 2013年10月17日 19:01 > >> To: Yang, Wenyou > >> Cc: linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; Ferre, > Nicolas; > >> Mark Brown > >> Subject: Re: [BUG] spi/spi-atmel: DMA rx buffer corruption with > >> SPI_IOC_MESSAGE > >> > >> 2013/10/8 Richard Genoud : > >> > Hi all, > >> > > >> > I finally found the bug I saw months ago, before "[PATCH v8 3/8] > >> > spi/spi-atmel: add dmaengine support" was merged. > >> > > >> > Here it is: > >> > > >> > When the ioctl SPI_IOC_MESSAGE is used with small and big buffers, > >> > the big RX buffer is corrupted with bytes from the big TX buffer. > >> > (Small means size < DMA_MIN_BYTES, Big means size > > >> DMA_MIN_BYTES) > >> > > >> > I'm attaching the test software that I used ( ./spi_test /dev/spidevx.x ) > >> > > >> > It fills 3 TX buffers with 0xAA pattern: a small, a big and a small > >> > again. > >> > It reads in return 3 RX buffers. > >> > The MISO pin has to be on 3.3v. > >> > > >> > It checks if the received buffers are filled with 0xFF (they should > >> > be, as MISO is high). > >> > And I've got a lot of buffers filled partially with 0xAA bytes. > >> > > >> > BUT, if you apply this patch: > >> > ---8< > >> > diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c > >> > index 83cf609..cde42a4 100644 > >> > --- a/drivers/spi/spi-atmel.c > >> > +++ b/drivers/spi/spi-atmel.c > >> > @@ -187,7 +187,7 @@ > >> > /* use PIO for small transfers, avoiding DMA setup/teardown > overhead > >> and > >> > * cache operations; better heuristics consider wordsize and > bitrate. > >> > */ > >> > -#define DMA_MIN_BYTES 16 > >> > +#define DMA_MIN_BYTES 0 > >> > > >> > struct atmel_spi_dma { > >> > struct dma_chan *chan_rx; > >> > ---8< > >> > There's no error any more. > >> > So there's something wrong happening when switching from/to pio > >> > transfer to/from DMA. > >> > > >> > We didn't saw that at the time of the merge because we did the test > >> > with a loop on MISO/MOSI, > >> > so the RX buffer was corrupted with identical bytes. > >> > > >> > Best regards, > >> > Richard. > >> > >> Hi Wenyou, > >> > >> Did you have the opportunity to give it a test ? > >> > >> Thread: https://lkml.org/lkml/2013/10/8/329 > > I tried to reproduce it with this thread, but I didn't > > Which slave device did you use? > > Hi Wenyou, > > I'm not using any slave device, I'm just setting the MISO pin to 3.3V, > then I execute the C code ( https://lkml.org/lkml/2013/10/8/329 ) > So, we should only receive 0xFF in the buffer. > But i'm getting : > aa aa aa aa aa aa aa aa aa aa aa aa aa aa ff ff > ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff > ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff > ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff > ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff > ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff > ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff > ff ff ff ff ff ff ff ff > aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa > aa aa aa aa aa aa aa aa aa aa aa aa aa aa ff ff > ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff > ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff > ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff > ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff > ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff > ff ff ff ff ff ff ff ff > Hi, Richard, The bug is reproduced. Tested on at91sam9g25ek, Linux-3.6.9. > Richard. Best Regards, Wenyou Yang
Re: [PATCH] f2fs: introduce f2fs_kmem_cache_alloc to hide the unfailed kmem cache allocation
Looks neat. Reviewed-by: Haicheng Li On Mon, Oct 21, 2013 at 03:24:55PM +0800, Gu Zheng wrote: > Introduce the unfailed version of kmem_cache_alloc named f2fs_kmem_cache_alloc > to hide the retry routine and make the code a bit cleaner. > > Signed-off-by: Gu Zheng > --- > fs/f2fs/checkpoint.c | 26 +++--- > fs/f2fs/f2fs.h | 13 + > fs/f2fs/gc.c |8 ++-- > fs/f2fs/node.c |6 +- > 4 files changed, 23 insertions(+), 30 deletions(-) > > diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c > index 8d16071..6fb484c 100644 > --- a/fs/f2fs/checkpoint.c > +++ b/fs/f2fs/checkpoint.c > @@ -226,12 +226,8 @@ void add_orphan_inode(struct f2fs_sb_info *sbi, nid_t > ino) > break; > orphan = NULL; > } > -retry: > - new = kmem_cache_alloc(orphan_entry_slab, GFP_ATOMIC); > - if (!new) { > - cond_resched(); > - goto retry; > - } > + > + new = f2fs_kmem_cache_alloc(orphan_entry_slab, GFP_ATOMIC); > new->ino = ino; > > /* add new_oentry into list which is sorted by inode number */ > @@ -484,12 +480,8 @@ void set_dirty_dir_page(struct inode *inode, struct page > *page) > > if (!S_ISDIR(inode->i_mode)) > return; > -retry: > - new = kmem_cache_alloc(inode_entry_slab, GFP_NOFS); > - if (!new) { > - cond_resched(); > - goto retry; > - } > + > + new = f2fs_kmem_cache_alloc(inode_entry_slab, GFP_NOFS); > new->inode = inode; > INIT_LIST_HEAD(>list); > > @@ -506,13 +498,9 @@ retry: > void add_dirty_dir_inode(struct inode *inode) > { > struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb); > - struct dir_inode_entry *new; > -retry: > - new = kmem_cache_alloc(inode_entry_slab, GFP_NOFS); > - if (!new) { > - cond_resched(); > - goto retry; > - } > + struct dir_inode_entry *new = > + f2fs_kmem_cache_alloc(inode_entry_slab, GFP_NOFS); > + > new->inode = inode; > INIT_LIST_HEAD(>list); > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > index 171c52f..fa9ad03 100644 > --- a/fs/f2fs/f2fs.h > +++ b/fs/f2fs/f2fs.h > @@ -787,6 +787,19 @@ static inline struct kmem_cache > *f2fs_kmem_cache_create(const char *name, > return kmem_cache_create(name, size, 0, SLAB_RECLAIM_ACCOUNT, ctor); > } > > +static inline void *f2fs_kmem_cache_alloc(struct kmem_cache *cachep, > + gfp_t flags) > +{ > + void *entry = kmem_cache_alloc(cachep, flags); > +retry: > + if (!entry) { > + cond_resched(); > + goto retry; > + } > + > + return entry; > +} > + > #define RAW_IS_INODE(p) ((p)->footer.nid == (p)->footer.ino) > > static inline bool IS_INODE(struct page *page) > diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c > index fbad968..7914b92 100644 > --- a/fs/f2fs/gc.c > +++ b/fs/f2fs/gc.c > @@ -361,12 +361,8 @@ static void add_gc_inode(struct inode *inode, struct > list_head *ilist) > iput(inode); > return; > } > -repeat: > - new_ie = kmem_cache_alloc(winode_slab, GFP_NOFS); > - if (!new_ie) { > - cond_resched(); > - goto repeat; > - } > + > + new_ie = f2fs_kmem_cache_alloc(winode_slab, GFP_NOFS); > new_ie->inode = inode; > list_add_tail(_ie->list, ilist); > } > diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c > index ef80f79..fe3cf8e 100644 > --- a/fs/f2fs/node.c > +++ b/fs/f2fs/node.c > @@ -1308,11 +1308,7 @@ static int add_free_nid(struct f2fs_nm_info *nm_i, > nid_t nid, bool build) > if (allocated) > return 0; > retry: > - i = kmem_cache_alloc(free_nid_slab, GFP_NOFS); > - if (!i) { > - cond_resched(); > - goto retry; > - } > + i = f2fs_kmem_cache_alloc(free_nid_slab, GFP_NOFS); > i->nid = nid; > i->state = NID_NEW; > > -- > 1.7.7 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] intel_microcode, Fix long microcode load time when firmware file is missing
On Mon, Oct 21, 2013 at 10:25 PM, Prarit Bhargava wrote: > > > On 10/21/2013 08:32 AM, Ming Lei wrote: >> On Mon, Oct 21, 2013 at 8:26 PM, Prarit Bhargava wrote: And why don't you pass FW_ACTION_HOTPLUG? and you are sure that udev isn't required to handle your microcode update request? >>> >>> AFAICT in both cases, udev wasn't required to handle the cpu microcode >>> update. >>> Both drivers use CMH to load the firmware which removes the need for udev >>> to do >>> anything. Admittedly maybe I've missed some odd use case but I don't think >>> it >>> is necessary. >> >> OK, so I guess the CMH still need uevent to get notified, right? > > The code as it is _currently_ written does not use uevents to load the > processor > firmware. ie) call_usermodehelper does not need uevent to get notified, so I > think FW_ACTION_NOHOTPLUG is correct. You need to make sure your patch won't break userspace in old distribution with your _currently_ code. Actually if udev isn't used in your user space, the timeout issue won't be triggered because it is blocked by udev. Thanks, -- Ming Lei -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: How to set fops in "struct platform_pwm_backlight_data"?
Ping Thierry... Mark On 10/18/2013 12:48 PM, Mark Zhang wrote: > On 10/17/2013 03:14 PM, Thierry Reding wrote: >> On Thu, Oct 17, 2013 at 02:49:57PM +0800, Mark Zhang wrote: >>> Hi, >>> >>> This is the first time I send mail to linux-pwm, I didn't read through >>> the mails in this list, so if somebody already asked this question, I'm >>> sorry about that. >>> >>> I wanna set some fops in "struct platform_pwm_backlight_data". But the >>> currrent probe function in pwm_bl.c says: >>> >>> --- >>> if (!data) { >>> ret = pwm_backlight_parse_dt(>dev, ); >>> if (ret < 0) { >>> dev_err(>dev, "failed to find platform data\n"); >>> return ret; >>> } >>> >>> data = >>> } >>> --- >>> >>> This looks like if we set the platform data for pwm backlight device, >>> "pwm_backlight_parse_dt" will never have a chance to be called, which >>> means the stuffs I defined in backlight DT node will be ignored. >>> >>> If I don't set the platform data for pwm backlight device, according to >>> the pwm_backlight_probe, I will never have a chance to set some fops >>> which I need(like "notify", "check_fb"...). >>> >>> So, what I suppose to do now? Maybe there is a way to set function >>> pointers in DT? >> >> Perhaps you could describe in more detail what you need the functions >> for. >> > > Okay, I just want to set the "notify" function pointer in "struct > platform_pwm_backlight_data", because I want to tune the brightness > value before the pwm-bl sets the brightness to hardware. I don't know > how to do that, unless we define the platform data explicitly. > > Mark >> Generally you're not supposed to mix DT and platform data. Without more >> info that's about all I can say. >> >> Thierry >> -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] NTB: remove duplicate defines
On Mon, Oct 21, 2013 at 07:19:42AM +0200, Michael Opdenacker wrote: > Remove duplicate defines in drivers/ntb/ntb_regs.h > > Signed-off-by: Michael Opdenacker Applied to my next tree, which should go into 3.13. Thanks, Jon > --- > drivers/ntb/ntb_regs.h | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/drivers/ntb/ntb_regs.h b/drivers/ntb/ntb_regs.h > index aa4bdd3..caa5975 100644 > --- a/drivers/ntb/ntb_regs.h > +++ b/drivers/ntb/ntb_regs.h > @@ -75,9 +75,6 @@ > #define SNB_SBAR2XLAT_OFFSET 0x0030 > #define SNB_SBAR4XLAT_OFFSET 0x0038 > #define SNB_SBAR0BASE_OFFSET 0x0040 > -#define SNB_SBAR0BASE_OFFSET 0x0040 > -#define SNB_SBAR2BASE_OFFSET 0x0048 > -#define SNB_SBAR4BASE_OFFSET 0x0050 > #define SNB_SBAR2BASE_OFFSET 0x0048 > #define SNB_SBAR4BASE_OFFSET 0x0050 > #define SNB_NTBCNTL_OFFSET 0x0058 > -- > 1.8.1.2 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] firmware, fix request_firmware_nowait() freeze with no uevent
On Tue, Oct 22, 2013 at 6:24 AM, Prarit Bhargava wrote: > > > On 10/21/2013 08:24 AM, Ming Lei wrote: >> On Mon, Oct 21, 2013 at 5:35 AM, Prarit Bhargava wrote: >>> If request_firmware_nowait() is called with uevent == NULL, the firmware >>> completion is never marked complete resulting in a hang in the process. >>> >>> If uevent is undefined, that means we're not waiting on anything and the >>> process should just clean up and complete. While we're at it, add a >>> debug dev_dbg() to indicate that the FW has not been found. >>> >>> Signed-off-by: Prarit Bhargava >>> Cc: x...@kernel.org >>> Cc: herrmann.der.u...@googlemail.com >>> Cc: ming@canonical.com >>> Cc: tig...@aivazian.fsnet.co.uk >>> --- >>> drivers/base/firmware_class.c |6 +- >>> 1 file changed, 5 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c >>> index 10a4467..95778dc 100644 >>> --- a/drivers/base/firmware_class.c >>> +++ b/drivers/base/firmware_class.c >>> @@ -335,7 +335,8 @@ static bool fw_get_filesystem_firmware(struct device >>> *device, >>> set_bit(FW_STATUS_DONE, >status); >>> complete_all(>completion); >>> mutex_unlock(_lock); >>> - } >>> + } else >>> + dev_dbg(device, "firmware: %s not found\n", buf->fw_id); >>> >>> return success; >>> } >>> @@ -886,6 +887,9 @@ static int _request_firmware_load(struct firmware_priv >>> *fw_priv, bool uevent, >>> schedule_delayed_work(_priv->timeout_work, >>> timeout); >>> >>> kobject_uevent(_priv->dev.kobj, KOBJ_ADD); >>> + } else { >>> + /* if there is no uevent then just cleanup */ >>> + schedule_delayed_work(_priv->timeout_work, 0); >>> } >> >> This may not a good idea and might break current NOHOTPLUG >> users, > > Ming, > > The code is broken for all callers of request_firmware_nowait() with NOHOTPLUG > and CONFIG_FW_LOADER_USER_HELPER=y. AFAICT with the two existing cases of > this > usage in the kernel, both are broken and both are attempting to do the same > thing that I'm doing in the x86 microcode ATM. > > This is the situation as I understand it and please correct me if I'm wrong > about the execution path. If I call request_firmware_nowait() with NOHOTPLUG > I > am essentially saying that there is no uevent associated with this firmware > load; that is uevent = 0. request_firmware_work_func() is called as scheduled > task, which results in a call to _request_firmware(). _request_firmware() > first > calls _request_firmware_prepare() which eventually results in a call to > __allocate_fw_buf() which does an init_completion(>completion). > > Returning back up the stack to _request_firmware() we eventually call > fw_get_filesystem_firmware(). _If the firmware does not exist_ success is > false > and the if (success) loop is not executed, and it is important to note that > the > complete_all(>completion) is _not_ called. fw_get_filesystem_firmware() > returns an error so that fw_load_from_user_helper() is called from > _request_firmware(). > > fw_load_from_user_helper() eventually calls _request_firmware_load() and this > is > where we get into a problem. fw_load_from_user_helper() calls all the file > creation, etc., and then hits this chunk of code: > > if (uevent) { > dev_set_uevent_suppress(f_dev, false); > dev_dbg(f_dev, "firmware: requesting %s\n", buf->fw_id); > if (timeout != MAX_SCHEDULE_TIMEOUT) > schedule_delayed_work(_priv->timeout_work, > timeout); > > kobject_uevent(_priv->dev.kobj, KOBJ_ADD); > } > > wait_for_completion(>completion); > > As I previously said, we've been called with NOHOTPLUG, ie) uevent = 0. That > means we skip down to the wait_for_completion(>completion) ... and we > wait > ... forever. Yes, it is exactly the previous design on NOHOTPLUG, because firmware loader has to wait for the handling from user space, and no one can predict when userspace comes because of no notification. For example, the userspace may be 'some inputting from shell by someone once he is free', :-) so it is difficult to set a timeout explicitly for the handling. But the requests can be killed before suspend & shutdown, so it is still OK. That is why NOHOTPLUG isn't encouraged to be taken, actually I don't suggest you to do that too, :-) You need to make sure your approach won't break micro-code update application in current/previous distributions. > > I can reproduce this by using a Dell PE 1850 & the dell_rbu module by doing > the > following: > > insmod dell_rbu.ko > echo init > /sys/devices/platform/dell_rbu/image_type > lsmod | grep dell_rbu > > (after an hour) > > [root@dell-pe1850-04 dell_rbu]# lsmod | grep dell_rbu > dell_rbu 14315 1 > [root@dell-pe1850-04 dell_rbu]# > > ^^^ that use count
Re: [PATCH] davinci_emac.c: Fix IFF_ALLMULTI setup
Hi Mariusz, On Mon, Oct 21, 2013 at 11:15 PM, Mariusz Ceier wrote: > When IFF_ALLMULTI flag is set on interface and IFF_PROMISC isn't, > emac_dev_mcast_set should only enable RX of multicasts and reset > MACHASH registers. > > It does this, but afterwards it either sets up multicast MACs > filtering or disables RX of multicasts and resets MACHASH registers > again, rendering IFF_ALLMULTI flag useless. > > This patch fixes emac_dev_mcast_set, so that multicast MACs filtering and > disabling of RX of multicasts are skipped when IFF_ALLMULTI flag is set. > > Tested with kernel 2.6.37. > It would have been better if you would have tested this on the latest kernel. Anyway David has pulled this patch in his tree. Regards, --Prabhakar Lad -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCHv5 4/4] Documentation: Add device tree bindings for Freescale FTM PWM.
Hi Rob, Pawel, Mark, Stephon, Ian Could someone help review and ACK the devicetree binding patch please ? Thanks very much! Best Regards, Xiubo > -Original Message- > From: Thierry Reding [mailto:thierry.red...@gmail.com] > Sent: Tuesday, October 08, 2013 9:57 PM > To: swar...@wwwdotorg.org; ian.campb...@citrix.com; mark.rutl...@arm.com; > pawel.m...@arm.com; rob.herr...@calxeda.com > Cc: Guo Shawn-R65073; s.ha...@pengutronix.de; t.f...@samsung.com; > grant.lik...@linaro.org; li...@arm.linux.org.uk; r...@landley.net; linux- > arm-ker...@lists.infradead.org; linux-...@vger.kernel.org; linux- > ker...@vger.kernel.org; devicet...@vger.kernel.org; linux- > d...@vger.kernel.org; Xiubo Li-B47053 > Subject: Re: [PATCHv5 4/4] Documentation: Add device tree bindings for > Freescale FTM PWM. > > On Mon, Sep 30, 2013 at 02:13:31PM +0800, Xiubo Li wrote: > > This adds the Document for Freescale FTM PWM driver under > > Documentation/devicetree/bindings/pwm/. > > > > Signed-off-by: Xiubo Li > > --- > > .../devicetree/bindings/pwm/pwm-fsl-ftm.txt| 33 > ++ > > 1 file changed, 33 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/pwm/pwm-fsl-ftm.txt > > Can someone of the device tree bindings maintainers review and ACK this? > I think it'd be good to squash this patch with the driver patch (patch 1 > of this series). I can do that when I apply this. > > Thierry > > > > > diff --git a/Documentation/devicetree/bindings/pwm/pwm-fsl-ftm.txt > > b/Documentation/devicetree/bindings/pwm/pwm-fsl-ftm.txt > > new file mode 100644 > > index 000..2c6969a > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/pwm/pwm-fsl-ftm.txt > > @@ -0,0 +1,33 @@ > > +Freescale FTM PWM controller > > + > > +Required properties: > > +- compatible: Should be "fsl,vf610-ftm-pwm" > > +- reg: Physical base address and length of the controller's registers > > +- #pwm-cells: Should be 3. See pwm.txt in this directory for a > > +description of > > + the cells format. > > +- clock-names : Includes the following module clock source entries: > > +"ftm0" (system clock), > > +"ftm0_fix_sel" (fixed frequency clock), > > +"ftm0_ext_sel" (external clock) > > +- clocks : Must contain a clock specifier for each entry in clock- > names. > > +- fsl,pwm-counter-clk: The FTM PWM counter clock source, should be > > +one of the > > + entries in clock-names. > > +- pinctrl-names: must contain a "default" entry. > > +- pinctrl-NNN: One property must exist for each entry in pinctrl-names. > > + See ../pinctrl/pinctrl-bindings.txt for details of the property > values. > > + > > + > > +Example: > > + > > +pwm0: pwm@40038000 { > > + compatible = "fsl,vf610-ftm-pwm"; > > + reg = <0x40038000 0x1000>; > > + #pwm-cells = <3>; > > + clock-names = "ftm0", "ftm0_fix_sel", "ftm0_ext_sel"; > > + clocks = < VF610_CLK_FTM0>, > > + < VF610_CLK_FTM0_FIX_SEL>, > > + < VF610_CLK_FTM0_EXT_SEL>; > > + pinctrl-names = "default"; > > + pinctrl-0 = <_pwm0_1>; > > + fsl,pwm-counter-clk = "ftm0_ext_sel"; }; > > -- > > 1.8.0 > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/1] [PATCH] ax88179_178a: Add VID:DID for Samsung USB Ethernet Adapter
From: Freddy Xin Add VID:DID for Samsung USB Ethernet Adapter. Signed-off-by: Freddy Xin --- drivers/net/usb/ax88179_178a.c | 19 ++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c index 3569293..3d166ae 100644 --- a/drivers/net/usb/ax88179_178a.c +++ b/drivers/net/usb/ax88179_178a.c @@ -1406,6 +1406,19 @@ static const struct driver_info sitecom_info = { .tx_fixup = ax88179_tx_fixup, }; +static const struct driver_info samsung_info = { + .description = "Samsung USB Ethernet Adapter", + .bind = ax88179_bind, + .unbind = ax88179_unbind, + .status = ax88179_status, + .link_reset = ax88179_link_reset, + .reset = ax88179_reset, + .stop = ax88179_stop, + .flags = FLAG_ETHER | FLAG_FRAMING_AX, + .rx_fixup = ax88179_rx_fixup, + .tx_fixup = ax88179_tx_fixup, +}; + static const struct usb_device_id products[] = { { /* ASIX AX88179 10/100/1000 */ @@ -1418,7 +1431,11 @@ static const struct usb_device_id products[] = { }, { /* Sitecom USB 3.0 to Gigabit Adapter */ USB_DEVICE(0x0df6, 0x0072), - .driver_info = (unsigned long) _info, + .driver_info = (unsigned long)_info, +}, { + /* Samsung USB Ethernet Adapter */ + USB_DEVICE(0x04e8, 0xa100), + .driver_info = (unsigned long)_info, }, { }, }; -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: linux-next: manual merge of the tip tree
On Thu, 17 Oct 2013 11:23:49 +0200 Peter Zijlstra wrote: > On Thu, Oct 17, 2013 at 12:28:59PM +1100, NeilBrown wrote: > > I always run with lockdep enabled, and I have done at least basic testing > > Very good! > > > > > > > Stuff like: > > > > > > + for (i = 0; i < NR_STRIPE_HASH_LOCKS; i++) > > > + spin_lock_init(conf->hash_locks + i); > > > > > > And: > > > > > > +static void __lock_all_hash_locks(struct r5conf *conf) > > > +{ > > > + int i; > > > + for (i = 0; i < NR_STRIPE_HASH_LOCKS; i++) > > > + spin_lock(conf->hash_locks + i); > > > +} > > > > > > Tends to complain real loud. > > > > Why is that? > > Because "conf->hash_locks + i" gets used as the "name" of the lockdep map > > for > > each one, and when they are all locked it looks like nested locking?? > > Exactly so; they all share the same class (and name) because they have > the same init site; so indeed the multiple lock will look like a nested > lock. > > > Do you have a suggestion for how to make this work? > > Would > > spin_lock_nested(conf->hash_locks + i, i) > > do the trick? > > spin_lock_nest_lock(conf->hash_locks + i, >device_lock); Unfortunately this doesn't work as the order is backwards. hash_lock is taken first, then (when necessary) device lock. (hash_lock is needed more often, so we split it up to reduce contention. device lock is needed less often, but sometimes when hash_lock is held). I've currently got: spin_lock_init(conf->hash_locks); for (i = 1; i < NR_STRIPE_HASH_LOCKS; i++) spin_lock_init(conf->hash_locks + i); and spin_lock(conf->hash_locks); for (i = 1; i < NR_STRIPE_HASH_LOCKS; i++) spin_lock_nest_lock(conf->hash_locks + i, conf->hash_locks); spin_lock(>device_lock); which doesn't trigger any lockdep warnings and isn't too ugly. Does it seem OK to you? Thanks, NeilBrown > > Would be the better option; your suggestion might just work because > NR_STRIP_HASH_LOCKS is 8 and we have exactly 8 subclasses available, but > any increase to NR_STRIPE_HASH_LOCKS will make things explode again. > > The spin_lock_nest_lock() annotation tells that the lock order is > irrelevant because all such multiple acquisitions are serialized under > the other lock. > > Also, if in future you feel the need to increase NR_STRIP_HASH_LOCKS, > please keep it <= 64 or so; if you have a need to go above that, please > yell and we'll see if we can do something smarter. I've added a comment to this effect in the code. > > This is because of: > - each spin_lock() increases preempt_count and that's 8 bits; we >wouldn't want to overflow that > - each consecutive nested spin_lock() increases the total acquisition >wait-time for all locks. Note that the worst case acquisition time >for even a single hash lock is gated by the complete acquisition time >of all of them in this scenario. signature.asc Description: PGP signature
[PATCH 1/1] [PATCH resubmit] ax88179_178a: Correct the RX error definition in RX header
From: Freddy Xin Correct the definition of AX_RXHDR_CRC_ERR and AX_RXHDR_DROP_ERR. They are BIT29 and BIT31 in pkt_hdr seperately. Signed-off-by: Freddy Xin --- drivers/net/usb/ax88179_178a.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c index 3569293..3bcd0d9 100644 --- a/drivers/net/usb/ax88179_178a.c +++ b/drivers/net/usb/ax88179_178a.c @@ -36,8 +36,8 @@ #define AX_RXHDR_L4_TYPE_TCP 16 #define AX_RXHDR_L3CSUM_ERR2 #define AX_RXHDR_L4CSUM_ERR1 -#define AX_RXHDR_CRC_ERR ((u32)BIT(31)) -#define AX_RXHDR_DROP_ERR ((u32)BIT(30)) +#define AX_RXHDR_CRC_ERR ((u32)BIT(29)) +#define AX_RXHDR_DROP_ERR ((u32)BIT(31)) #define AX_ACCESS_MAC 0x01 #define AX_ACCESS_PHY 0x02 #define AX_ACCESS_EEPROM 0x04 -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Add SMP support for Allwinner A20: PATCH V5 1/3] Add smp support for Allwinner A20(sunxi 7i).
HI Maxime, How about the new V5 patch? Fan Thanks On 18 October 2013 00:37, Fan Rong wrote: > This patch adds SMP support for the Allwinner A20 SoC. This SoC uses an IP > to, among other things, handle the CPU-related configuration, like the power > clamp, the boot address of the secondary CPUS, etc. We thus need to map this > IP during the prepare_cpu SMP operation, before bringing up the secondary CPU > in the secondary_startup operation. > > Signed-off-by: Fan Rong > --- > arch/arm/mach-sunxi/Makefile | 2 + > arch/arm/mach-sunxi/headsmp.S | 18 +++ > arch/arm/mach-sunxi/platsmp.c | 114 > ++ > arch/arm/mach-sunxi/sunxi.c | 3 ++ > 4 files changed, 137 insertions(+) > create mode 100644 arch/arm/mach-sunxi/headsmp.S > create mode 100644 arch/arm/mach-sunxi/platsmp.c > mode change 100644 => 100755 arch/arm/mach-sunxi/sunxi.c > > diff --git a/arch/arm/mach-sunxi/Makefile b/arch/arm/mach-sunxi/Makefile > index 93bebfc..d7f1ef4 100644 > --- a/arch/arm/mach-sunxi/Makefile > +++ b/arch/arm/mach-sunxi/Makefile > @@ -1 +1,3 @@ > obj-$(CONFIG_ARCH_SUNXI) += sunxi.o > +obj-$(CONFIG_ARCH_SUNXI) += platsmp.o > +obj-$(CONFIG_ARCH_SUNXI) += headsmp.o > diff --git a/arch/arm/mach-sunxi/headsmp.S b/arch/arm/mach-sunxi/headsmp.S > new file mode 100644 > index 000..48c9d33 > --- /dev/null > +++ b/arch/arm/mach-sunxi/headsmp.S > @@ -0,0 +1,18 @@ > +/* > + * SMP support for A20 > + * > + * Copyright (C) 2013 Fan Rong > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + */ > + > +#include > +#include > + > +.section ".text.head", "ax" > +ENTRY(sun7i_secondary_startup) > + msr cpsr_fsxc,#0xd3 > + b secondary_startup > +ENDPROC(sun7i_secondary_startup) > diff --git a/arch/arm/mach-sunxi/platsmp.c b/arch/arm/mach-sunxi/platsmp.c > new file mode 100644 > index 000..fa5adde > --- /dev/null > +++ b/arch/arm/mach-sunxi/platsmp.c > @@ -0,0 +1,114 @@ > +/* > + * linux/arch/arm/mach-sun7i/platsmp.c > + * > + * Copyright (C) 2013 Fan Rong > + * All Rights Reserved > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + */ > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > + > +/* > + * CPU Configure module support > + * 1: Software reset for smp cpus > + * 2: Configure for smp cpus including boot. > + * 3: Three 64-bit idle counters and two 64-bit common counters > + * it is needed for smp cpus > + */ > +void __iomem *sun7i_cc_base; /*CPU Configure Base*/ > +extern void sun7i_secondary_startup(void); > + > +/* > + * CPUCFG > + */ > +#define SUN7I_CPUCFG_BOOTADDR 0x01a4 > + > +#define SUN7I_CPUCFG_GENCTL0x0184 > +#define SUN7I_CPUCFG_DBGCTL0 0x01e0 > +#define SUN7I_CPUCFG_DBGCTL1 0x01e4 > + > +#define SUN7I_CPU1_PWR_CLAMP 0x01b0 > +#define SUN7I_CPU1_PWROFF_REG 0x01b4 > +#define SUN7I_CPUX_RESET_CTL(x)(0x40 + (x)*0x40) > + > +static struct of_device_id sun7i_cc_ids[] = { > + { .compatible = "allwinner,sun7i-a20-cpuconfig"}, > + { /*sentinel*/ } > +}; > + > +static int sun7i_boot_secondary(unsigned int cpu, struct task_struct *idle) > +{ > + long paddr; > + uint32_t pwr_reg; > + uint32_t j = 0xff << 1; > + if (!sun7i_cc_base) { > + pr_debug("error map cpu configure\n"); > + return -ENOSYS; > + } > + /* Set boot addr */ > + paddr = virt_to_phys(sun7i_secondary_startup); > + writel(paddr, sun7i_cc_base + SUN7I_CPUCFG_BOOTADDR); > + > + /* Assert cpu core reset */ > + writel(0, sun7i_cc_base + SUN7I_CPUX_RESET_CTL(cpu)); > + > + /* Ensure CPU reset also invalidates L1 caches */ > + pwr_reg = readl(sun7i_cc_base + SUN7I_CPUCFG_GENCTL); > + pwr_reg &= ~BIT(cpu); > + writel(pwr_reg, sun7i_cc_base + SUN7I_CPUCFG_GENCTL); > + > + /* DBGPWRDUP hold low */ > + pwr_reg = readl(sun7i_cc_base + SUN7I_CPUCFG_DBGCTL1); > + pwr_reg &= ~BIT(cpu); > + writel(pwr_reg, sun7i_cc_base + SUN7I_CPUCFG_DBGCTL1); > + > + /* Ramp up power to CPU1 */ > + do { > + writel(j, sun7i_cc_base + SUN7I_CPU1_PWR_CLAMP); > + j = j >> 1; > + } while (j != 0); > + > + mdelay(10); > + > + pwr_reg = readl(sun7i_cc_base + SUN7I_CPU1_PWROFF_REG); > + pwr_reg &= ~1; > + writel(pwr_reg, sun7i_cc_base + SUN7I_CPU1_PWROFF_REG); > + mdelay(1); > + > + /* Release CPU reset */ > + writel(3, sun7i_cc_base + SUN7I_CPUX_RESET_CTL(cpu)); > + > + /* Unlock CPU */ > + pwr_reg = readl(sun7i_cc_base + SUN7I_CPUCFG_DBGCTL1); > + pwr_reg |= BIT(cpu); > +
Re: [PATCH] cpufreq: acpi: Add comment under ACPI_ADR_SPACE_SYSTEM_IO case
On 22 October 2013 04:09, Rafael J. Wysocki wrote: > I don't think I've missed it. It should be commit 1bab64d in my tree. I fetched your tree yesterday and this wasn't there in linux-next or bleeding-edge branch.. When I fetched it now again, it is there.. Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xfs: fix possible NULL dereference
On Mon, Oct 21, 2013 at 07:00:59PM -0500, Eric Sandeen wrote: > On 10/21/13 6:56 PM, Dave Chinner wrote: > > On Mon, Oct 21, 2013 at 06:18:49PM -0500, Ben Myers wrote: > >> Hey, > >> > >> On Mon, Oct 21, 2013 at 06:12:18PM -0500, Eric Sandeen wrote: > >>> On 10/21/13 5:44 PM, Dave Chinner wrote: > On Mon, Oct 21, 2013 at 03:58:23PM -0500, Eric Sandeen wrote: > > On 10/21/13 1:32 PM, Geyslan G. Bem wrote: > >> This patch puts a 'break' in the true branch, avoiding the > >> 'icptr->ic_next' > >> dereferencing. > > > > Reviewed-by: Eric Sandeen > > Actually, NACK. > >>> > >>> I felt that one coming ;) > >>> > > Hm, yeah - cmn_err(CE_PANIC, " " ); used to BUG_ON, but the newer > > xfs_emerg() doesn't. > > > > Dave, was that intentional? > > Of course it was. ;) xfs_emerg() is only called from the debug code > in xlog_verify_iclog(), xlog_verify_tail_lsn and assfail(). > > In the case of assfail(), it has it's own BUG() call, so it does > everything just fine. > > In the case of xlog_verify_iclog() when icptr is NULL, it will > panic immediately after the message is printed, just like the old > code. i.e. this patch isn't fixing anything we need fixed. > >>> > >>> A BUG() is probably warranted, then. > >> > >> I tend to agree with Eric on this point. If we want to crash, I'd rather > >> our > >> intent be very clear, rather than just see a null ptr deref. ;) > > > > Sure. ASSERT() would be better and more consistent with the rest of > > the code. i.e: > > > > for (i = 0; i < log->l_iclog_bufs; i++, icptr = icptr->ic_next) > > ASSERT(icptr); > > > > > > > > However, I keep coming back to the fact that what we are checking is > > that the list is correctly circular and that and adding an > > ASSERT(icptr) to panic if a pointer chase finds a null pointer is > > kinda redundant, especially as: > > > > - there's already 2 comments for the function indicating > > that it is checking the validity of the pointers and that > > they are circular > > - we have repeatedly, over many years, justified the removal > > of ASSERT(ptr) from code like: > > > > ASSERT(ptr); > > foo = ptr->foo; > > > > as it is redundant because production code will always > > panic the machine in that situation via the dereference. > > ASSERT() is for documenting assumptions and constraints > > that are not obvious from the code context. > > > > IOWs, in this case the presence or absence of the ASSERT inside > > *debug-only code* doesn't add any addition value to debugging such > > problems, nor does it add any value in terms of documentation > > because it's clear from the comments in the debug code that it > > should not be NULL to begin with. > > > > > > I guess what's left as unclear is why we would prefer to panic > vs. handling the error, even if it's in debug code. The caller can > handle errors, so blowing up here sure doesn't look intentional. If the iclog list is corrupt and not circular, then we will simply panic the next time it is iterated. Lots of log code iterates the iclog list, such as log IO completion, switching iclogs, log forces, etc. > Maybe the answer is it's debug code > and we want to drop to the debugger or generate a vmcore at that > point, but that's just been demonstrated as quite unclear to the > casual reader. :) Yes, but to continue the Devil's Advocate argument, the purpose of debug code isn't to enlightent the casual reader or drive-by patchers - it's to make life easier for people who actually spend time debugging the code. And the people who need the debug code are expected to understand why an ASSERT is not necessary. :) Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/4] drivers/misc: add rawio framework and drivers
To read/write registers from a device is very important on embedded system, especially SoC systems. Physically there could be different types of devices based on bus tyes, e.g. PCI devices, I2C (slave)devices, I/O devices(memory mapped), inter-processor devices, etc. Typically there are userland tools from PC Linux to access device registers, but on some embedded system initrd and rootfs come with a minimal busybox and most useful userland tools are not available. To add these tools back to rootfs is not convenient either. What's more, on some systems with runtime pm enabled, reading/writing registers from a device which is in low power state will cause problems. For these reasons, to have some tools/interfaces directly from kernel space via debug fs seems to be easy, cheap and convenient. These patchsets are designed to achieve above goals to ease device driver and kernel debugging on embedded systems. Rawio provides a framework to read/write registers from a bus, including pci, i2c, I/O device(memory mapped), etc. based on debug fs. Rawio bus drivers implement the read/write operation on a specific bus on top of the rawio framework driver. Currently only three bus drivers are available: pci, iomem and i2c. But it's extremely easy to add more drivers on top of the framework if needed. drivers/misc/Kconfig | 1 + drivers/misc/Makefile| 1 + drivers/misc/rawio/Kconfig | 59 + drivers/misc/rawio/Makefile | 4 + drivers/misc/rawio/rawio.c | 514 +++ drivers/misc/rawio/rawio_i2c.c | 224 + drivers/misc/rawio/rawio_iomem.c | 401 ++ drivers/misc/rawio/rawio_pci.c | 235 ++ include/linux/rawio.h| 78 ++ 9 files changed, 1517 insertions(+) create mode 100644 drivers/misc/rawio/Kconfig create mode 100644 drivers/misc/rawio/Makefile create mode 100644 drivers/misc/rawio/rawio.c create mode 100644 drivers/misc/rawio/rawio_i2c.c create mode 100644 drivers/misc/rawio/rawio_iomem.c create mode 100644 drivers/misc/rawio/rawio_pci.c create mode 100644 include/linux/rawio.h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 4/4] drivers/misc: add rawio i2c driver
With i2c rawio driver, you can read/write registers from any i2c client(slave) device via debug fs interface. This driver is based on the rawio framework. Signed-off-by: Bin Gao --- drivers/misc/rawio/Kconfig | 11 ++ drivers/misc/rawio/Makefile| 1 + drivers/misc/rawio/rawio_i2c.c | 224 + 3 files changed, 236 insertions(+) create mode 100644 drivers/misc/rawio/rawio_i2c.c diff --git a/drivers/misc/rawio/Kconfig b/drivers/misc/rawio/Kconfig index 38e8a52..9ffbc5d 100644 --- a/drivers/misc/rawio/Kconfig +++ b/drivers/misc/rawio/Kconfig @@ -45,4 +45,15 @@ config RAWIO_IOMEM To compile this driver as a module, choose M: the module will be called rawio_iomem. +config RAWIO_I2C + tristate "rawio I2C driver" + depends on RAWIO && I2C + default no + help + This option enables the rawio I2C driver. + With this driver, you can read or write any I2C device's + register debugfs interface. + To compile this driver as a module, choose M: the module will + be called rawio_i2c. + endif # RAWIO diff --git a/drivers/misc/rawio/Makefile b/drivers/misc/rawio/Makefile index 5f86257..9e33aec 100644 --- a/drivers/misc/rawio/Makefile +++ b/drivers/misc/rawio/Makefile @@ -1,3 +1,4 @@ obj-$(CONFIG_RAWIO)+= rawio.o obj-$(CONFIG_RAWIO_PCI)+= rawio_pci.o obj-$(CONFIG_RAWIO_IOMEM) += rawio_iomem.o +obj-$(CONFIG_RAWIO_I2C)+= rawio_i2c.o diff --git a/drivers/misc/rawio/rawio_i2c.c b/drivers/misc/rawio/rawio_i2c.c new file mode 100644 index 000..f872602 --- /dev/null +++ b/drivers/misc/rawio/rawio_i2c.c @@ -0,0 +1,224 @@ +/* + * rawio_i2c.c - rawio I2C driver. + * Read or write a I2C device's register, based on the rawio framework. + * + * Copyright (c) 2013 Bin Gao + * + * This file is released under the GPLv2 + * + * + * read i2c registers: + * echo "r i2c[]" > + * /sys/kernel/debug/rawio_cmd + * cat /sys/kernel/debug/rawio_output + * e.g. echo "r i2c 3 0x6b 0x84 5" > /sys/kernel/debug/rawio_cmd + * cat /sys/kernel/debug/rawio_output + * + * write a i2c register: + * echo "w i2c" > + * /sys/kernel/debug/rawio_cmd + * cat /sys/kernel/debug/rawio_output + * e.g. echo "w i2c 4 0x70 0x4 0xfa" > /sys/kernel/debug/rawio_cmd + * cat /sys/kernel/debug/rawio_output + * + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +static int i2c_prepare(u8 i2c_bus, u16 i2c_addr, u16 i2c_reg, u16 len, + int ten_bit_addr, struct i2c_adapter **ppadap) +{ + struct i2c_adapter *adap; + + adap = i2c_get_adapter((int)i2c_bus); + if (!adap) { + rawio_err("can't find bus adapter for i2c bus %d\n", + i2c_bus); + return -ENODEV; + } + + if ((!ten_bit_addr && (i2c_addr > 128)) || (i2c_addr > 1024)) { + rawio_err("register address is out of range, forgot 't' for 10bit addr?\n"); + return -EINVAL; + } + + if ((!ten_bit_addr && ((i2c_addr + len) > 128)) || + ((i2c_addr + len) > 1024)) { + rawio_err("register address is out of range, forgot 't' for 10bit addr?\n"); + return -EINVAL; + } + + *ppadap = adap; + return 0; +} + +static int rawio_i2c_read(struct rawio_driver *driver, int width, u64 *input, + u8 *postfix, int input_num, void **output, int *output_num) +{ + int ret, len; + struct i2c_adapter *adap; + u16 i2c_addr, i2c_reg; + struct i2c_msg msg[2]; + u8 i2c_bus, buf[2], *out_buf, ten_bit_addr, sixteen_bit_reg; + + i2c_bus = (u8)input[0]; + i2c_addr = (u16)input[1]; + i2c_reg = (u16)input[2]; + + len = 1; + if (input_num == 4) + len = (u16)input[3]; + + ten_bit_addr = postfix[1]; + sixteen_bit_reg = postfix[2]; + + ret = i2c_prepare(i2c_bus, i2c_addr, i2c_reg, len, ten_bit_addr, ); + if (ret) + return ret; + + out_buf = kzalloc(sizeof(u8) * len, GFP_KERNEL); + if (buf == NULL) { + rawio_err("can't alloc memory\n"); + return -ENOMEM; + } + buf[0] = i2c_reg & 0xff; + buf[1] = (i2c_reg >> 8) & 0xff; + + /* write i2c reg address */ + msg[0].addr = i2c_addr; + msg[0].flags = ten_bit_addr ? I2C_M_TEN : 0; + msg[0].len = sixteen_bit_reg ? 2 : 1; + msg[0].buf = buf; + + /* read i2c reg */ + msg[1].addr = i2c_addr; + msg[1].flags = I2C_M_RD | (ten_bit_addr ? I2C_M_TEN : 0); + msg[1].len = len; + msg[1].buf = out_buf; + + ret = i2c_transfer(adap, msg, 2); + if (ret != 2) { + rawio_err("i2c_transfer() failed, ret = %d\n", ret); + kfree(out_buf); + return -EIO; + } + +
[PATCH 3/4] drivers/misc: add rawio iomem driver
With iomem rawio driver, you can read/write memory mapped registers from any I/O device via debug fs interface. This driver is based on the rawio framework. Signed-off-by: Bin Gao --- drivers/misc/rawio/Kconfig | 12 ++ drivers/misc/rawio/Makefile | 1 + drivers/misc/rawio/rawio_iomem.c | 401 +++ 3 files changed, 414 insertions(+) create mode 100644 drivers/misc/rawio/rawio_iomem.c diff --git a/drivers/misc/rawio/Kconfig b/drivers/misc/rawio/Kconfig index 47be40a..38e8a52 100644 --- a/drivers/misc/rawio/Kconfig +++ b/drivers/misc/rawio/Kconfig @@ -33,4 +33,16 @@ config RAWIO_PCI To compile this driver as a module, choose M: the module will be called rawio_pci. +config RAWIO_IOMEM + tristate "rawio I/O memory driver" + depends on RAWIO + default no + help + This option enables the rawio I/O memory driver. + With this driver, you can read or write registers of a memory + mapped I/O devices. + + To compile this driver as a module, choose M: the module will + be called rawio_iomem. + endif # RAWIO diff --git a/drivers/misc/rawio/Makefile b/drivers/misc/rawio/Makefile index 0933ca6..5f86257 100644 --- a/drivers/misc/rawio/Makefile +++ b/drivers/misc/rawio/Makefile @@ -1,2 +1,3 @@ obj-$(CONFIG_RAWIO)+= rawio.o obj-$(CONFIG_RAWIO_PCI)+= rawio_pci.o +obj-$(CONFIG_RAWIO_IOMEM) += rawio_iomem.o diff --git a/drivers/misc/rawio/rawio_iomem.c b/drivers/misc/rawio/rawio_iomem.c new file mode 100644 index 000..eaae773 --- /dev/null +++ b/drivers/misc/rawio/rawio_iomem.c @@ -0,0 +1,401 @@ +/* + * rawio_iomem.c - a driver to read or write a device's I/O memory, based on + * the rawio debugfs framework. + * + * Copyright (c) 2013 Bin Gao + * + * This file is released under the GPLv2 + * + * + * 1: byte, 2: word, 4: dword + * + * I/O mem read: + * echo "r[1|2|4] iomem []" > /sys/kernel/debug/rawio_cmd + * cat /sys/kernel/debug/rawio_output + * e.g. echo "r iomem 0xff003040 20" > /sys/kernel/debug/rawio_cmd + * cat /sys/kernel/debug/rawio_output + * + * I/O mem write: + * echo "w[1|2|4] iomem " > /sys/kernel/debug/rawio_cmd + * cat /sys/kernel/debug/rawio_output + * e.g. echo "w2 iomem 0xff003042 0xb03f" > /sys/kernel/debug/rawio_cmd + * cat /sys/kernel/debug/rawio_output + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +/* + * On some platforms, a read or write to a device which is in a low power + * state will cause a system error, or even cause a system reboot. + * To address this, we use runtime PM APIs to bring up the device to + * runnint state before use and put back to original state after use. + * + * We could use lookup_resource() to map an physical address to a device, + * but there are some problems: + * 1) lookup_resource() is not exported, so a kernel module can't use it; + * 2) To use the 'name' field of 'struct resource' to match a device is + *not reliable; + * So we rather walk known device types than look up the resrouce list + * to map a physical address(I/O memory address) to a device. + */ + +/* return true if range(start: m2, size: n2) is in range(start: m1, size: n1) */ +#define IN_RANGE(m1, n1, m2, n2) \ + (((m2 >= m1) && (m2 < (m1 + n1))) && \ + (((m2 + n2) >= m1) && ((m2 + n2) < (m1 + n1 + +struct dev_walker { + resource_size_t addr; + resource_size_t size; + struct device *dev; + int error; +}; + +#ifdef CONFIG_PCI +int walk_pci_devices(struct device *dev, void *data) +{ + int i; + resource_size_t start, len; + struct pci_dev *pdev = (struct pci_dev *) to_pci_dev(dev); + struct dev_walker *walker = (struct dev_walker *) data; + + if (!pdev) + return -ENODEV; + + walker->dev = NULL; + for (i = 0; i < 6; i++) { + start = pci_resource_start(pdev, i); + len = pci_resource_len(pdev, i); + if (IN_RANGE(start, len, walker->addr, walker->size)) { + walker->dev = dev; + return 1; + } + } + + return 0; +} +#endif + +int walk_platform_devices(struct device *dev, void *data) +{ + int i; + struct resource *r; + resource_size_t start, len; + struct platform_device *plat_dev = to_platform_device(dev); + struct dev_walker *walker = (struct dev_walker *) data; + + walker->dev = NULL; + for (i = 0; i < plat_dev->num_resources; i++) { + r = platform_get_resource(plat_dev, IORESOURCE_MEM, i); + if (!r) + continue; + start = r->start; + len = r->end - r->start + 1; + if (IN_RANGE(start, len, walker->addr, walker->size)) { + walker->dev =
[PATCH 2/4] drivers/misc: add rawio pci driver
With pci rawio driver, you can read/write any pci config space register by debug fs interface. This driver is based on the rawio framework. Signed-off-by: Bin Gao --- drivers/misc/rawio/Kconfig | 15 +++ drivers/misc/rawio/Makefile| 1 + drivers/misc/rawio/rawio_pci.c | 235 + 3 files changed, 251 insertions(+) create mode 100644 drivers/misc/rawio/rawio_pci.c diff --git a/drivers/misc/rawio/Kconfig b/drivers/misc/rawio/Kconfig index fd4272e..47be40a 100644 --- a/drivers/misc/rawio/Kconfig +++ b/drivers/misc/rawio/Kconfig @@ -19,3 +19,18 @@ menuconfig RAWIO be called rawio. If you are not sure, say N here. + +if RAWIO + +config RAWIO_PCI + tristate "rawio PCI driver" + depends on RAWIO && PCI + default no + help + This option enables the rawio PCI driver. + With this driver, you can read or write any PCI device's + configuration space via debugfs. + To compile this driver as a module, choose M: the module will + be called rawio_pci. + +endif # RAWIO diff --git a/drivers/misc/rawio/Makefile b/drivers/misc/rawio/Makefile index c21453c..0933ca6 100644 --- a/drivers/misc/rawio/Makefile +++ b/drivers/misc/rawio/Makefile @@ -1 +1,2 @@ obj-$(CONFIG_RAWIO)+= rawio.o +obj-$(CONFIG_RAWIO_PCI)+= rawio_pci.o diff --git a/drivers/misc/rawio/rawio_pci.c b/drivers/misc/rawio/rawio_pci.c new file mode 100644 index 000..052ad1b --- /dev/null +++ b/drivers/misc/rawio/rawio_pci.c @@ -0,0 +1,235 @@ +/* + * rawio_pci.c - a driver to read/write pci configuration space registers based + * on the rawio framework. + * + * 1: byte, 2: word, 4: dword + * + * read pci config space registers + * echo "r[1|2|4] pci []" > + * /sys/kernel/debug/rawio_cmd + * cat /sys/kernel/debug/rawio_output + * e.g. echo "r1 pci 0 0 3 0 8 12" > /sys/kernel/debug/rawio_cmd + * cat /sys/kernel/debug/rawio_output + * + * write a pci config space register: + * echo "w[1|2|4] pci " > + * /sys/kernel/debug/rawio_output + * cat /sys/kernel/debug/rawio_output + * e.g. echo "w pci 0 0 0x11 2 0x10 0x" > /sys/kernel/debug/rawio_cmd + * cat /sys/kernel/debug/rawio_output + * + * + * Copyright (c) 2013 Bin Gao + * + * This file is released under the GPLv2 + * + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +static int pci_prepare(int pci_domain, unsigned int pci_bus, + u8 pci_dev, u8 pci_func, enum width width, + u16 pci_reg, u16 len, struct pci_dev **ppdev) +{ + struct pci_dev *pdev; + int ret; + + if (((width == WIDTH_2) && (pci_reg & 0x1)) || + ((width == WIDTH_4) && (pci_reg & 0x3))) { + rawio_err("register address requires 2 bytes aligned for 16 bit access, and 4 bytes aligned for 32 bit access\n"); + return -EINVAL; + } + + pdev = pci_get_domain_bus_and_slot(pci_domain, pci_bus, + PCI_DEVFN(pci_dev, pci_func)); + if (!pdev) { + rawio_err("pci device %04x:%02x:%02x.%01x doesn't exist\n", + pci_domain, pci_bus, pci_dev, pci_func); + return -ENODEV; + } + + if (((pci_reg >= 0x100) && !pci_is_pcie(pdev)) || + (pci_reg >= 0x1000)) { + rawio_err("register address is out of range\n"); + return -EINVAL; + } + + if pci_reg + len * width) >= 0x100) && !pci_is_pcie(pdev)) || + ((pci_reg + len * width) >= 0x1000)) { + rawio_err("register address is out of range\n"); + return -EINVAL; + } + + ret = pm_runtime_get_sync(>dev); + if ((ret >= 0) || (ret == -EACCES)) + goto out; + + rawio_err("can't put pci device %04x:%02x:%02x.%01xinto running state, pm_runtime_get_sync() returned %d\n", + pci_domain, pci_bus, pci_dev, pci_func, ret); + return -EBUSY; + +out: + *ppdev = pdev; + return 0; +} + +static void pci_finish(struct pci_dev *pdev) +{ + pm_runtime_put_sync(>dev); +} + +static int rawio_pci_read(struct rawio_driver *driver, int width, + u64 *input, u8 *postfix, int input_num, + void **output, int *output_num) +{ + int i, ret, pci_domain; + struct pci_dev *pdev; + unsigned int pci_bus; + u8 pci_dev, pci_func; + u16 pci_reg, len; + void *buf; + + pci_domain = (int)input[0]; + pci_bus = (unsigned int)input[1]; + pci_dev = (u8)input[2]; + pci_func = (u8)input[3]; + pci_reg = (u16)input[4]; + len = 1; + if (input_num == 6) + len = (u16)input[5]; + + ret = pci_prepare(pci_domain, pci_bus, pci_dev,
Re: [PATCH] xfs: fix possible NULL dereference
On 10/21/13 6:56 PM, Dave Chinner wrote: > On Mon, Oct 21, 2013 at 06:18:49PM -0500, Ben Myers wrote: >> Hey, >> >> On Mon, Oct 21, 2013 at 06:12:18PM -0500, Eric Sandeen wrote: >>> On 10/21/13 5:44 PM, Dave Chinner wrote: On Mon, Oct 21, 2013 at 03:58:23PM -0500, Eric Sandeen wrote: > On 10/21/13 1:32 PM, Geyslan G. Bem wrote: >> This patch puts a 'break' in the true branch, avoiding the >> 'icptr->ic_next' >> dereferencing. > > Reviewed-by: Eric Sandeen Actually, NACK. >>> >>> I felt that one coming ;) >>> > Hm, yeah - cmn_err(CE_PANIC, " " ); used to BUG_ON, but the newer > xfs_emerg() doesn't. > > Dave, was that intentional? Of course it was. ;) xfs_emerg() is only called from the debug code in xlog_verify_iclog(), xlog_verify_tail_lsn and assfail(). In the case of assfail(), it has it's own BUG() call, so it does everything just fine. In the case of xlog_verify_iclog() when icptr is NULL, it will panic immediately after the message is printed, just like the old code. i.e. this patch isn't fixing anything we need fixed. >>> >>> A BUG() is probably warranted, then. >> >> I tend to agree with Eric on this point. If we want to crash, I'd rather our >> intent be very clear, rather than just see a null ptr deref. ;) > > Sure. ASSERT() would be better and more consistent with the rest of > the code. i.e: > > for (i = 0; i < log->l_iclog_bufs; i++, icptr = icptr->ic_next) > ASSERT(icptr); > > > > However, I keep coming back to the fact that what we are checking is > that the list is correctly circular and that and adding an > ASSERT(icptr) to panic if a pointer chase finds a null pointer is > kinda redundant, especially as: > > - there's already 2 comments for the function indicating > that it is checking the validity of the pointers and that > they are circular > - we have repeatedly, over many years, justified the removal > of ASSERT(ptr) from code like: > > ASSERT(ptr); > foo = ptr->foo; > > as it is redundant because production code will always > panic the machine in that situation via the dereference. > ASSERT() is for documenting assumptions and constraints > that are not obvious from the code context. > > IOWs, in this case the presence or absence of the ASSERT inside > *debug-only code* doesn't add any addition value to debugging such > problems, nor does it add any value in terms of documentation > because it's clear from the comments in the debug code that it > should not be NULL to begin with. > > I guess what's left as unclear is why we would prefer to panic vs. handling the error, even if it's in debug code. The caller can handle errors, so blowing up here sure doesn't look intentional. Maybe the answer is it's debug code and we want to drop to the debugger or generate a vmcore at that point, but that's just been demonstrated as quite unclear to the casual reader. :) -Eric > Cheers, > > Dave. > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/4] drivers/misc: add rawio framework driver
Rawio provides a framework to read/write registers from a bus, including pci, i2c, I/O device(memory mapped), etc. based on debug fs. Rawio bus drivers implement the read/write operation on a specific bus on top of the rawio framework driver. They are designed to help device driver and kernel debugging on embedded systems. Signed-off-by: Bin Gao --- drivers/misc/Kconfig| 1 + drivers/misc/Makefile | 1 + drivers/misc/rawio/Kconfig | 21 ++ drivers/misc/rawio/Makefile | 1 + drivers/misc/rawio/rawio.c | 514 include/linux/rawio.h | 78 +++ 6 files changed, 616 insertions(+) create mode 100644 drivers/misc/rawio/Kconfig create mode 100644 drivers/misc/rawio/Makefile create mode 100644 drivers/misc/rawio/rawio.c create mode 100644 include/linux/rawio.h diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index 8dacd4c..1afbe4e 100644 --- a/drivers/misc/Kconfig +++ b/drivers/misc/Kconfig @@ -537,4 +537,5 @@ source "drivers/misc/carma/Kconfig" source "drivers/misc/altera-stapl/Kconfig" source "drivers/misc/mei/Kconfig" source "drivers/misc/vmw_vmci/Kconfig" +source "drivers/misc/rawio/Kconfig" endmenu diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index c235d5b..3bc116b 100644 --- a/drivers/misc/Makefile +++ b/drivers/misc/Makefile @@ -53,3 +53,4 @@ obj-$(CONFIG_INTEL_MEI) += mei/ obj-$(CONFIG_VMWARE_VMCI) += vmw_vmci/ obj-$(CONFIG_LATTICE_ECP3_CONFIG) += lattice-ecp3-config.o obj-$(CONFIG_SRAM) += sram.o +obj-$(CONFIG_RAWIO)+= rawio/ diff --git a/drivers/misc/rawio/Kconfig b/drivers/misc/rawio/Kconfig new file mode 100644 index 000..fd4272e --- /dev/null +++ b/drivers/misc/rawio/Kconfig @@ -0,0 +1,21 @@ +# +# rawio utility drivers +# + +menuconfig RAWIO + tristate "Debug fs based raw io device read/write framework " + depends on DEBUG_FS + default no + help + This option enables support for reading or writing registers/memory + region in a io device via debug fs. + With this option and related rawio driver options enabled, you could + read configuration space of a PCI device, registers of a memory + mapped or port mapped device, registers of a i2c device, etc. + This is the just the framework driver. You need enable more + options to support specific device types. + + To compile this driver as a module, choose M: the module will + be called rawio. + + If you are not sure, say N here. diff --git a/drivers/misc/rawio/Makefile b/drivers/misc/rawio/Makefile new file mode 100644 index 000..c21453c --- /dev/null +++ b/drivers/misc/rawio/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_RAWIO)+= rawio.o diff --git a/drivers/misc/rawio/rawio.c b/drivers/misc/rawio/rawio.c new file mode 100644 index 000..a05b493 --- /dev/null +++ b/drivers/misc/rawio/rawio.c @@ -0,0 +1,514 @@ +/* + * rawio.c - a debugfs based framework for reading/writing registers + * from a I/O device. + * With pluggable rawio drivers, it can support PCI devices, I2C devices, + * memory mapped I/O devices, etc. + * It's designed for helping debug Linux device drivers on embedded system or + * SoC platforms. + * + * Copyright (c) 2013 Bin Gao + * + * This file is released under the GPLv2 + * + * + * Two files are created in debugfs root folder: rawio_cmd and rawio_output. + * To read or write via the rawio debugfs interface, first echo a rawio + * command to the file rawio_cmd, then cat the file rawio_output: + * $ echo "" > /sys/kernel/debug/rawio_cmd + * $ cat /sys/kernel/debug/rawio_output + * The cat command is required for both read and write operations. + * For details of rawio command format, see specific rawio drivers. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define SHOW_NUM_PER_LINE (32 / active_width) +#define LINE_WIDTH 32 +#define IS_WHITESPACE(c) ((c) == ' ' || (c) == '\t' || (c) == '\n') + +static struct dentry *rawio_cmd_dentry, *rawio_output_dentry; +static char rawio_cmd_buf[RAWIO_CMD_LEN], rawio_err_buf[RAWIO_ERR_LEN + 1]; +static DEFINE_MUTEX(rawio_lock); +static LIST_HEAD(rawio_driver_head); +static struct rawio_driver *active_driver; +static enum width active_width; +static enum ops active_ops; +static u64 args_val[RAWIO_ARGS_MAX]; +static u8 args_postfix[RAWIO_ARGS_MAX]; +static int num_args_val; + +static void store_value(u64 *where, void *value, enum type type) +{ + switch (type) { + case TYPE_U8: + *(u8 *)where = *(u8 *)value; + break; + case TYPE_U16: + *(u16 *)where = *(u16 *)value; + break; + case TYPE_U32: + *(u32 *)where = *(u32 *)value; + break; + case TYPE_U64: + *where = *(u64 *)value; +
Re: [PATCH] xfs: fix possible NULL dereference
On Mon, Oct 21, 2013 at 06:18:49PM -0500, Ben Myers wrote: > Hey, > > On Mon, Oct 21, 2013 at 06:12:18PM -0500, Eric Sandeen wrote: > > On 10/21/13 5:44 PM, Dave Chinner wrote: > > > On Mon, Oct 21, 2013 at 03:58:23PM -0500, Eric Sandeen wrote: > > >> On 10/21/13 1:32 PM, Geyslan G. Bem wrote: > > >>> This patch puts a 'break' in the true branch, avoiding the > > >>> 'icptr->ic_next' > > >>> dereferencing. > > >> > > >> Reviewed-by: Eric Sandeen > > > > > > Actually, NACK. > > > > I felt that one coming ;) > > > > >> Hm, yeah - cmn_err(CE_PANIC, " " ); used to BUG_ON, but the newer > > >> xfs_emerg() doesn't. > > >> > > >> Dave, was that intentional? > > > > > > Of course it was. ;) xfs_emerg() is only called from the debug code > > > in xlog_verify_iclog(), xlog_verify_tail_lsn and assfail(). > > > > > > In the case of assfail(), it has it's own BUG() call, so it does > > > everything just fine. > > > > > > In the case of xlog_verify_iclog() when icptr is NULL, it will > > > panic immediately after the message is printed, just like the old > > > code. i.e. this patch isn't fixing anything we need fixed. > > > > A BUG() is probably warranted, then. > > I tend to agree with Eric on this point. If we want to crash, I'd rather our > intent be very clear, rather than just see a null ptr deref. ;) Sure. ASSERT() would be better and more consistent with the rest of the code. i.e: for (i = 0; i < log->l_iclog_bufs; i++, icptr = icptr->ic_next) ASSERT(icptr); However, I keep coming back to the fact that what we are checking is that the list is correctly circular and that and adding an ASSERT(icptr) to panic if a pointer chase finds a null pointer is kinda redundant, especially as: - there's already 2 comments for the function indicating that it is checking the validity of the pointers and that they are circular - we have repeatedly, over many years, justified the removal of ASSERT(ptr) from code like: ASSERT(ptr); foo = ptr->foo; as it is redundant because production code will always panic the machine in that situation via the dereference. ASSERT() is for documenting assumptions and constraints that are not obvious from the code context. IOWs, in this case the presence or absence of the ASSERT inside *debug-only code* doesn't add any addition value to debugging such problems, nor does it add any value in terms of documentation because it's clear from the comments in the debug code that it should not be NULL to begin with. Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] tpm: use tabs instead of whitespaces in Kconfig
just like the other entries Signed-off-by: Peter Huewe --- drivers/char/tpm/Kconfig | 16 1 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig index 4716af5..4dd4363 100644 --- a/drivers/char/tpm/Kconfig +++ b/drivers/char/tpm/Kconfig @@ -101,14 +101,14 @@ config TCG_IBMVTPM as a module, choose M here; the module will be called tpm_ibmvtpm. config TCG_ST33_I2C -tristate "STMicroelectronics ST33 I2C TPM" -depends on I2C -depends on GPIOLIB ----help--- -If you have a TPM security chip from STMicroelectronics working with -an I2C bus say Yes and it will be accessible from within Linux. -To compile this driver as a module, choose M here; the module will be -called tpm_i2c_stm_st33. + tristate "STMicroelectronics ST33 I2C TPM" + depends on I2C + depends on GPIOLIB + ---help--- + If you have a TPM security chip from STMicroelectronics working with + an I2C bus say Yes and it will be accessible from within Linux. + To compile this driver as a module, choose M here; the module will be + called tpm_i2c_stm_st33. config TCG_XEN tristate "XEN TPM Interface" -- 1.7.5.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] tpm: Fix module name description in Kconfig for tpm_i2c_*
This patch changes the displayed module name from tpm_tis_i2c_infineon to its actual name tpm_i2c_infineon and from tpm_stm_st33_i2c to tpm_i2c_stm_st33 Signed-off-by: Peter Huewe --- drivers/char/tpm/Kconfig |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig index f908586..4716af5 100644 --- a/drivers/char/tpm/Kconfig +++ b/drivers/char/tpm/Kconfig @@ -51,7 +51,7 @@ config TCG_TIS_I2C_INFINEON Specification 0.20 say Yes and it will be accessible from within Linux. To compile this driver as a module, choose M here; the module - will be called tpm_tis_i2c_infineon. + will be called tpm_i2c_infineon. config TCG_TIS_I2C_NUVOTON tristate "TPM Interface Specification 1.2 Interface (I2C - Nuvoton)" @@ -108,7 +108,7 @@ config TCG_ST33_I2C If you have a TPM security chip from STMicroelectronics working with an I2C bus say Yes and it will be accessible from within Linux. To compile this driver as a module, choose M here; the module will be -called tpm_stm_st33_i2c. +called tpm_i2c_stm_st33. config TCG_XEN tristate "XEN TPM Interface" -- 1.7.5.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Aw: Re: [tpmdd-devel] TPM patches for 3.12
Hi Jason, Ashley, I just pushed the changes to my github tree https://github.com/PeterHuewe/linux-tpmdd/commits/for-james along with some additional changes: - before the rename I made tpm.c checkpatch clean ;) - I did some cleanup on the indention / white spaces of both Kconfig and bindings for both new drivers - corrected the module name for the nuvoton i2c module I detected similar problems with other entries in the kconfig so I - did also correct the module name for the infineon i2c module (d'oh, my fault ;) - and also corrected it for the stm i2c module - cleaned the indention on the st i2c module I'm currently compile testing it again, I definitely need a newer/faster (small/portable) laptop (donations welcome ;) - e.g. a 11" ultrabook ;) Jason, can you please review and test my changes - I'd probably send the pull request out tomorrow then. (giving me also the chance to think again whether I should add myself to MAINTAINERS or not ;) Thanks, Peter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/3] tty: xuartps: Fix build error due to missing forward declaration
If CONFIG_PM_SLEEP is enabled and CONFIG_SERIAL_XILINX_PS_UART_CONSOLE is not, a forward declaration of the uart_driver struct is not included, leading to a build error due to an undeclared variable. Fixing this by moving the definition of the struct uart_driver before the definition of the suspend/resume callbacks. Signed-off-by: Soren Brinkmann --- drivers/tty/serial/xilinx_uartps.c | 28 ++-- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c index 5ac6c480df43..ca4a2f1fbca9 100644 --- a/drivers/tty/serial/xilinx_uartps.c +++ b/drivers/tty/serial/xilinx_uartps.c @@ -1198,6 +1198,20 @@ console_initcall(xuartps_console_init); #endif /* CONFIG_SERIAL_XILINX_PS_UART_CONSOLE */ +/** Structure Definitions + */ +static struct uart_driver xuartps_uart_driver = { + .owner = THIS_MODULE, /* Owner */ + .driver_name= XUARTPS_NAME, /* Driver name */ + .dev_name = XUARTPS_TTY_NAME, /* Node name */ + .major = XUARTPS_MAJOR,/* Major number */ + .minor = XUARTPS_MINOR,/* Minor number */ + .nr = XUARTPS_NR_PORTS, /* Number of UART ports */ +#ifdef CONFIG_SERIAL_XILINX_PS_UART_CONSOLE + .cons = _console, /* Console */ +#endif +}; + #ifdef CONFIG_PM_SLEEP /** * xuartps_suspend - suspend event @@ -1311,20 +1325,6 @@ static int xuartps_resume(struct device *device) static SIMPLE_DEV_PM_OPS(xuartps_dev_pm_ops, xuartps_suspend, xuartps_resume); -/** Structure Definitions - */ -static struct uart_driver xuartps_uart_driver = { - .owner = THIS_MODULE, /* Owner */ - .driver_name= XUARTPS_NAME, /* Driver name */ - .dev_name = XUARTPS_TTY_NAME, /* Node name */ - .major = XUARTPS_MAJOR,/* Major number */ - .minor = XUARTPS_MINOR,/* Minor number */ - .nr = XUARTPS_NR_PORTS, /* Number of UART ports */ -#ifdef CONFIG_SERIAL_XILINX_PS_UART_CONSOLE - .cons = _console, /* Console */ -#endif -}; - /* - * Platform bus binding */ -- 1.8.4.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/3] tty: xuartps: Fix build
This should fix the issues in the x86 build of the xuartps driver. The issues are: - [tty:tty-next 72/75] drivers/tty/serial/xilinx_uartps.c:436:7: error: 'PRE_RATE_CHANGE' undeclared - [tty:tty-next 73/75] drivers/tty/serial/xilinx_uartps.c:1227:21: error: 'xuartps_uart_driver' undeclared Additionally I found some build warnings while fixing those issues. There are some additional comments regarding patches 1 and 3 in their respective email. This series is based on the tty-next tree. Thanks, Sören Soren Brinkmann (3): tty: xuartps: Fix "may be used uninitialized" build warning tty: xuartps: Fix build error due to missing forward declaration tty: xuartps: Fix build error when COMMON_CLK is not set drivers/tty/serial/xilinx_uartps.c | 38 +++--- 1 file changed, 23 insertions(+), 15 deletions(-) -- 1.8.4.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/3] tty: xuartps: Fix "may be used uninitialized" build warning
Initialize varibles for which a 'may be used uninitalized' warning is issued. Signed-off-by: Soren Brinkmann --- The warning is actually a false positive. The variables are passed to a function per reference. That function uses those variables to return values, which then are used by the caller. Is there a way to avoid the initialization and the build warning alltogether? --- drivers/tty/serial/xilinx_uartps.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c index c7c96c2f149c..5ac6c480df43 100644 --- a/drivers/tty/serial/xilinx_uartps.c +++ b/drivers/tty/serial/xilinx_uartps.c @@ -389,7 +389,7 @@ static unsigned int xuartps_set_baud_rate(struct uart_port *port, unsigned int baud) { unsigned int calc_baud; - u32 cd, bdiv; + u32 cd = 0, bdiv = 0; u32 mreg; int div8; struct xuartps *xuartps = port->private_data; -- 1.8.4.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/3] tty: xuartps: Fix build error when COMMON_CLK is not set
Clock notifiers are only available when CONFIG_COMMON_CLK is enabled. Hence all notifier related code has to be protected by corresponsing ifdefs. Signed-off-by: Soren Brinkmann --- Alternatively this could be fixed by adding a dependency on COMMON_CLK in Kconfig. That would keep the code cleaner, but limit this driver to platforms using the CCF. --- drivers/tty/serial/xilinx_uartps.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c index ca4a2f1fbca9..e46e9f3f19b9 100644 --- a/drivers/tty/serial/xilinx_uartps.c +++ b/drivers/tty/serial/xilinx_uartps.c @@ -411,6 +411,7 @@ static unsigned int xuartps_set_baud_rate(struct uart_port *port, return calc_baud; } +#ifdef CONFIG_COMMON_CLK /** * xuartps_clk_notitifer_cb - Clock notifier callback * @nb:Notifier block @@ -504,6 +505,7 @@ static int xuartps_clk_notifier_cb(struct notifier_block *nb, return NOTIFY_DONE; } } +#endif /*--Uart Operations---*/ @@ -1380,11 +1382,13 @@ static int xuartps_probe(struct platform_device *pdev) goto err_out_clk_disable; } +#ifdef CONFIG_COMMON_CLK xuartps_data->clk_rate_change_nb.notifier_call = xuartps_clk_notifier_cb; if (clk_notifier_register(xuartps_data->refclk, _data->clk_rate_change_nb)) dev_warn(>dev, "Unable to register clock notifier.\n"); +#endif /* Initialize the port structure */ port = xuartps_get_port(); @@ -1415,8 +1419,10 @@ static int xuartps_probe(struct platform_device *pdev) } err_out_notif_unreg: +#ifdef CONFIG_COMMON_CLK clk_notifier_unregister(xuartps_data->refclk, _data->clk_rate_change_nb); +#endif err_out_clk_disable: clk_disable_unprepare(xuartps_data->refclk); err_out_clk_dis_aper: @@ -1438,8 +1444,10 @@ static int xuartps_remove(struct platform_device *pdev) int rc; /* Remove the xuartps port from the serial core */ +#ifdef CONFIG_COMMON_CLK clk_notifier_unregister(xuartps_data->refclk, _data->clk_rate_change_nb); +#endif rc = uart_remove_one_port(_uart_driver, port); port->mapbase = 0; clk_disable_unprepare(xuartps_data->refclk); -- 1.8.4.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [BUG] 3.12.0-rcX IPv6 panic
On Mon, Oct 21, 2013 at 05:52:52PM +0200, Hannes Frederic Sowa wrote: > On Mon, Oct 21, 2013 at 08:18:46AM -0500, Bob Tracy wrote: > > Actually, a regression: the 3.11 kernel is rock-solid stable on my > > Alpha. > > > > Beginning with 3.12.0-rc1, I can reliably trigger a kernel panic by > > executing the gogo6.net "gw6c" IPv6 client program. If the networking > > layer is active, an "Oops" will eventually (within a day) occur regardless > > of whether I attempt to run "gw6c". 3.12.0-rcX is stable as long as I > > leave networking completely disabled. The error has persisted up through > > -rc6. Apologies for not mentioning this earlier, but the state of my > > PWS-433au has been questionable, and I wanted to make sure I had a > > legitimate bug sighting. > > > > I'll have to transcribe the panic backtrace by hand: nothing makes it > > into any of the system logs :-(. I *can* recall that every backtrace > > I've seen thus far has included one of the skb_copy() variants near the > > top of the list (first or second function). > > Try to capture the panic via serial console. Otherwise a picture > would give us a first hint. Please watch out for lines like > skb_(over|under)_panic. I'll get a screen capture of some kind for you within the next day or so. > gw6c is a tunnel client? Can you post ip -6 tunnel ls? Assuming you meant "show [NAME]" (no "ls" option for the tunnel object), that yields the following with "gw6c" running on a 3.11.0 kernel: smirkin:~# ip -6 tunnel show sit1 sit1: any/ipv6 remote 4056:5874:: local 4500:0:0:4000:4029:: encaplimit 0 hoplimit 0 tclass 0x00 flowlabel 0x0 (flowinfo 0x) I'm running the gw6c client in gateway mode: the Alpha is my IPv6 gateway/firewall. --Bob -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] md: avoid deadlock when md_set_badblocks.
On Sat, 12 Oct 2013 01:10:03 -0400 ycbzzj...@gmail.com wrote: > From: Bian Yu > > When operate harddisk and hit errors, md_set_badblocks is called after > scsi_restart_operations which already disabled the irq. but md_set_badblocks > will call write_sequnlock_irq and enable irq. so softirq can preempt the > current thread and that may cause a deadlock. I think this situation should > use write_sequnlock_irqsave/irqrestore instead. > > I met the situation and the call trace is below: > [ 638.919974] BUG: spinlock recursion on CPU#0, scsi_eh_13/1010 > [ 638.921923] lock: 0x8800d4d51fc8, .magic: dead4ead, .owner: > scsi_eh_13/1010, .owner_cpu: 0 > [ 638.923890] CPU: 0 PID: 1010 Comm: scsi_eh_13 Not tainted 3.12.0-rc5+ #37 > [ 638.925844] Hardware name: To be filled by O.E.M. To be filled by > O.E.M./MAHOBAY, BIOS 4.6.5 03/05/2013 > [ 638.927816] 880037ad4640 880118c03d50 8172ff85 > 0007 > [ 638.929829] 8800d4d51fc8 880118c03d70 81730030 > 8800d4d51fc8 > [ 638.931848] 81a72eb0 880118c03d90 81730056 > 8800d4d51fc8 > [ 638.933884] Call Trace: > [ 638.935867][] dump_stack+0x55/0x76 > [ 638.937878] [] spin_dump+0x8a/0x8f > [ 638.939861] [] spin_bug+0x21/0x26 > [ 638.941836] [] do_raw_spin_lock+0xa4/0xc0 > [ 638.943801] [] _raw_spin_lock+0x66/0x80 > [ 638.945747] [] ? scsi_device_unbusy+0x9d/0xd0 > [ 638.947672] [] ? _raw_spin_unlock+0x2b/0x50 > [ 638.949595] [] scsi_device_unbusy+0x9d/0xd0 > [ 638.951504] [] scsi_finish_command+0x37/0xe0 > [ 638.953388] [] scsi_softirq_done+0xa8/0x140 > [ 638.955248] [] blk_done_softirq+0x7b/0x90 > [ 638.957116] [] __do_softirq+0xfd/0x330 > [ 638.958987] [] ? __lock_release+0x6f/0x100 > [ 638.960861] [] call_softirq+0x1c/0x30 > [ 638.962724] [] do_softirq+0x8d/0xc0 > [ 638.964565] [] irq_exit+0x10e/0x150 > [ 638.966390] [] smp_apic_timer_interrupt+0x4a/0x60 > [ 638.968223] [] apic_timer_interrupt+0x6f/0x80 > [ 638.970079][] ? __lock_release+0x6f/0x100 > [ 638.971899] [] ? _raw_spin_unlock_irq+0x3a/0x50 > [ 638.973691] [] ? _raw_spin_unlock_irq+0x30/0x50 > [ 638.975475] [] md_set_badblocks+0x1f3/0x4a0 > [ 638.977243] [] rdev_set_badblocks+0x27/0x80 > [ 638.978988] [] raid5_end_read_request+0x36b/0x4e0 > [raid456] > [ 638.980723] [] bio_endio+0x1d/0x40 > [ 638.982463] [] req_bio_endio.isra.65+0x83/0xa0 > [ 638.984214] [] blk_update_request+0x7f/0x350 > [ 638.985967] [] blk_update_bidi_request+0x31/0x90 > [ 638.987710] [] __blk_end_bidi_request+0x20/0x50 > [ 638.989439] [] __blk_end_request_all+0x1f/0x30 > [ 638.991149] [] blk_peek_request+0x106/0x250 > [ 638.992861] [] ? scsi_kill_request.isra.32+0xe9/0x130 > [ 638.994561] [] scsi_request_fn+0x4a/0x3d0 > [ 638.996251] [] __blk_run_queue+0x37/0x50 > [ 638.997900] [] blk_run_queue+0x2f/0x50 > [ 638.999553] [] scsi_run_queue+0xe0/0x1c0 > [ 639.001185] [] scsi_run_host_queues+0x21/0x40 > [ 639.002798] [] scsi_restart_operations+0x177/0x200 > [ 639.004391] [] scsi_error_handler+0xc9/0xe0 > [ 639.005996] [] ? scsi_unjam_host+0xd0/0xd0 > [ 639.007600] [] kthread+0xdb/0xe0 > [ 639.009205] [] ? flush_kthread_worker+0x170/0x170 > [ 639.010821] [] ret_from_fork+0x7c/0xb0 > [ 639.012437] [] ? flush_kthread_worker+0x170/0x170 > > Signed-off-by: Bian Yu > Reviewed-by: Jianpeng Ma > --- > drivers/md/md.c |5 +++-- > 1 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/md/md.c b/drivers/md/md.c > index 8a0d762..2445fec 100644 > --- a/drivers/md/md.c > +++ b/drivers/md/md.c > @@ -8111,6 +8111,7 @@ static int md_set_badblocks(struct badblocks *bb, > sector_t s, int sectors, > u64 *p; > int lo, hi; > int rv = 1; > + unsigned long flags; > > if (bb->shift < 0) > /* badblocks are disabled */ > @@ -8125,7 +8126,7 @@ static int md_set_badblocks(struct badblocks *bb, > sector_t s, int sectors, > sectors = next - s; > } > > - write_seqlock_irq(>lock); > + write_seqlock_irqsave(>lock, flags); > > p = bb->page; > lo = 0; > @@ -8241,7 +8242,7 @@ static int md_set_badblocks(struct badblocks *bb, > sector_t s, int sectors, > bb->changed = 1; > if (!acknowledged) > bb->unacked_exist = 1; > - write_sequnlock_irq(>lock); > + write_sequnlock_irqrestore(>lock, flags); > > return rv; > } Applied, thanks. NeilBrown signature.asc Description: PGP signature
Re: [PATCH] RFC: interrupt consistency check for OF GPIO IRQs
Hi Stephen, On Sunday 20 October 2013 22:35:04 Stephen Warren wrote: > On 10/20/2013 01:41 PM, Laurent Pinchart wrote: > > On Tuesday 17 September 2013 17:36:32 Grant Likely wrote: > >> On Thu, 12 Sep 2013 17:57:00 +0200, Alexander Holler wrote: > >>> Am 12.09.2013 17:19, schrieb Stephen Warren: > IRQs, DMA channels, and GPIOs are all different things. Their bindings > are defined independently. While it's good to define new types of > bindings consistently with other bindings, this hasn't always happened, > so you can make zero assumptions about the IRQ bindings by reading the > documentation for any other kind of binding. > > Multiple interrupts are defined as follows: > // Optional; otherwise inherited from parent/grand-parent/... > interrupt-parent = <>; > // Must be in a fixed order, unless binding defines that the > // optional interrupt-names property is to be used. > interrupts = <1 IRQF_TRIGGER_HIGH> <2 IRQF_TRIGGER_LOW>; > // Optional; binding for device defines whether it must > // be present > interrupt-names = "foo", "bar"; > > If you need multiple interrupts, each with a different parent, you need > to use an interrupt-map property... > > ... > > >> Actually, I think it is solveable but doing so requires a new binding > >> for interrupts. I took a shot at implementing it earlier this week and > >> I've got working patches that I'll be posting soon. I created a new > >> "interrupts-extended" property that uses a phandle+args type of > > >> binding like this: > ... > > >> device@3000 { > >>interrupts-extended = < 5> < 3 4> < 6>; > >> }; > > ... > > > Any progress on this ? I'll need to use multiple interrupts with different > > parents in the near future, I can take this over if needed. > > > > I've also been thinking that we could possibly reuse the "interrupts" > > property without defining a new "interrupts-extended". When parsing the > > property the code would use the current DT bindings if an > > interrupt-parent is present, and the new DT bindings if it isn't. > > interrupt-parents doesn't have to be present in individual nodes; it can > be inherited from the parent. That means you'd have to convert whole > sub-trees at once. Very good point. I agree with you, a new property is then better. > It seems much more flexible to use a new property and hence make it explicit > what format the data is in. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xfs: fix possible NULL dereference
Hey, On Mon, Oct 21, 2013 at 06:12:18PM -0500, Eric Sandeen wrote: > On 10/21/13 5:44 PM, Dave Chinner wrote: > > On Mon, Oct 21, 2013 at 03:58:23PM -0500, Eric Sandeen wrote: > >> On 10/21/13 1:32 PM, Geyslan G. Bem wrote: > >>> This patch puts a 'break' in the true branch, avoiding the > >>> 'icptr->ic_next' > >>> dereferencing. > >> > >> Reviewed-by: Eric Sandeen > > > > Actually, NACK. > > I felt that one coming ;) > > >> Hm, yeah - cmn_err(CE_PANIC, " " ); used to BUG_ON, but the newer > >> xfs_emerg() doesn't. > >> > >> Dave, was that intentional? > > > > Of course it was. ;) xfs_emerg() is only called from the debug code > > in xlog_verify_iclog(), xlog_verify_tail_lsn and assfail(). > > > > In the case of assfail(), it has it's own BUG() call, so it does > > everything just fine. > > > > In the case of xlog_verify_iclog() when icptr is NULL, it will > > panic immediately after the message is printed, just like the old > > code. i.e. this patch isn't fixing anything we need fixed. > > A BUG() is probably warranted, then. I tend to agree with Eric on this point. If we want to crash, I'd rather our intent be very clear, rather than just see a null ptr deref. ;) Regards, Ben -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RESEND PATCH v2 6/6] mmc: sdhci-bcm-kona: Add basic use of clocks
On Thu, Oct 17, 2013 at 7:13 AM, Mark Rutland wrote: > On Wed, Oct 16, 2013 at 10:47:10PM +0100, Tim Kryger wrote: >> Enable the external clock needed by the host controller during the >> probe and disable it during the remove. > > This requires a biding document update. The current best practice for declaring the association of a clock with a consumer device is to embed the common clock bindings 'clocks' property inside its node but this is not the only way it may be done. Given that clk_get() still works for clocks found using string matching, is it appropriate to mandate that all drivers that uses the clk api mention the common clock bindings 'clocks' property in the documentation for their device's binding? > I note that the binding is already incomplete (it does not describe the > interrupts or reg). The current document reflects the standard for Documentation/devicetree/bindings/mmc where normal properties are described in mmc.txt and only the deviations described in vendor specific files. >> >> Signed-off-by: Tim Kryger >> Reviewed-by: Markus Mayer >> Reviewed-by: Matt Porter >> --- >> drivers/mmc/host/sdhci-bcm-kona.c | 30 -- >> 1 file changed, 28 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/mmc/host/sdhci-bcm-kona.c >> b/drivers/mmc/host/sdhci-bcm-kona.c >> index 85472d3..91db099 100644 >> --- a/drivers/mmc/host/sdhci-bcm-kona.c >> +++ b/drivers/mmc/host/sdhci-bcm-kona.c >> @@ -54,6 +54,7 @@ >> >> struct sdhci_bcm_kona_dev { >> struct mutexwrite_lock; /* protect back to back writes */ >> + struct clk *external_clk; > > Is this the only clock input the unit has? The SDHCI block only receives one clock. > Are there any other external resources the device needs (e.g. gpios, > regulators)? Yes but the cd-gpio and vmmc/vqmmc regulators are handled by the common SDHCI driver. > >> }; >> >> >> @@ -252,11 +253,29 @@ static int sdhci_bcm_kona_probe(struct platform_device >> *pdev) >> mmc_of_parse(host->mmc); >> >> if (!host->mmc->f_max) { >> - dev_err(>dev, "Missing max-freq for SDHCI cfg\n"); >> + dev_err(dev, "Missing max-freq for SDHCI cfg\n"); > > This seems like an unrelated change. Agreed. I will remove this change. >> ret = -ENXIO; >> goto err_pltfm_free; >> } >> >> + /* Get and enable the external clock */ >> + kona_dev->external_clk = devm_clk_get(dev, NULL); >> + if (IS_ERR(kona_dev->external_clk)) { >> + dev_err(dev, "Failed to get external clock\n"); >> + ret = PTR_ERR(kona_dev->external_clk); >> + goto err_pltfm_free; > > This seems like a pretty clear breakage of existing DTBs. > > Why? The defconfig that builds this driver and DTBs currently sets CONFIG_ARM_APPENDED_DTB=y so there isn't any actual breakage risk. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xfs: fix possible NULL dereference
On 10/21/13 5:44 PM, Dave Chinner wrote: > On Mon, Oct 21, 2013 at 03:58:23PM -0500, Eric Sandeen wrote: >> On 10/21/13 1:32 PM, Geyslan G. Bem wrote: >>> This patch puts a 'break' in the true branch, avoiding the 'icptr->ic_next' >>> dereferencing. >> >> Reviewed-by: Eric Sandeen > > Actually, NACK. I felt that one coming ;) >> Hm, yeah - cmn_err(CE_PANIC, " " ); used to BUG_ON, but the newer >> xfs_emerg() doesn't. >> >> Dave, was that intentional? > > Of course it was. ;) xfs_emerg() is only called from the debug code > in xlog_verify_iclog(), xlog_verify_tail_lsn and assfail(). > > In the case of assfail(), it has it's own BUG() call, so it does > everything just fine. > > In the case of xlog_verify_iclog() when icptr is NULL, it will > panic immediately after the message is printed, just like the old > code. i.e. this patch isn't fixing anything we need fixed. A BUG() is probably warranted, then. -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Standing for the Technical Advisory Board
I'm standing for election to the Linux Foundation Technical Advisory Board. I plan to attend the Wednesday-evening event that will host the TAB election. I work in many different areas of the Linux and Open Source community, at all levels of the stack, including the kernel, plumbing, applications, services, distributions, and packaging. My first Open Source contribution, accepted ten years ago this month, was to OpenOffice.org, to the cross-distribution ooo-build patchset that later became LibreOffice, to make it build without the then-proprietary Java so it could enter Debian main: https://lists.debian.org/debian-devel-changes/2003/10/msg02847.html Not long after that, I started contributing to XCB, and in the process of helping to replace Xlib I became a de-facto co-maintainer of Xlib through the "last sucker to touch it" rule, thus showing a pattern of stepping up to address long-standing problems, and/or demonstrating the requisite degree of insanity. I maintained the Sparse static analysis tool for several years, making me one of two people to take over maintainership of a project from Linus. I'm one of the (thankfully growing) handful of people in the world to understand how RCU works, and I have the dissertation to prove it. (I think it says a lot about RCU that attempting to make it more approachable resulted in a three-sentence set of usage guidelines and a hundred pages of supporting material.) I currently maintain the rcutorture test module and the ACPI BGRT driver. I've also made many community contributions that don't directly involve writing code. I'm one of the most prolific patch reviewers in the kernel, going by Reviewed-by tags. I've written significant parts of Documentation/CodingStyle, documenting previously unwritten bits of kernel-community tribal knowledge that many developers would mention in patch NAKs but none could find a reference for. And I've provided mentorship for various projects by students or interns to contribute to the Linux kernel and the X Window System, including student "capstone" projects, Google Summer of Code interns, and Outreach Program for Women interns. I currently work as Intel's ChromeOS architect. Contributing across an entire Linux distribution, and to an Open Source commercial product based on Linux, provides a view not just into how things fit together but how they often don't. I've had first-hand experience with the tension between shipping a product and creating the right technical solution, as well as what happens when the kernel or low-level userspace breaks assumptions made higher up the stack. I enjoy working with many different parts of a system to produce a satisfying result that's more cohesive than any one project or patch can achieve alone; I'm standing for the TAB as a natural extension of that approach. In the spirit of Rusty Russell's perennial talk slide on "better things you could be learning about rather than attending this talk", other people standing for election that I plan to vote for and would encourage others to vote for as well: Jonathan Corbet: I can't think of a better representative for the community than the editor-in-chief of LWN. Sarah Sharp: Sarah maintains a a major Linux subsystem, represents the needs of Linux and Open Source to industry and standards groups, and works tirelessly to help new developers join the kernel community. Matthew Garrett: Matthew has extensive experience making hardware, firmware, and software play nice with each other, as well as experience working with people and organizations to make that possible and help keep it that way. Greg Kroah-Hartman: Greg maintains critical pieces of core kernel infrastructure, acts as a stalwart front-line defender of kernel standards and quality, and helps many prospective driver authors learn the give-and-take required to successfully participate in the community. - Josh Triplett -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mac802154: correct a typo in ieee802154_alloc_device() prototype
From: Alexandre Belloni Date: Mon, 21 Oct 2013 19:09:58 +0200 > This has no other impact than a cosmetic one. > > Signed-off-by: Alexandre Belloni Applied. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] davinci_emac.c: Fix IFF_ALLMULTI setup
From: Mariusz Ceier Date: Mon, 21 Oct 2013 19:45:04 +0200 > When IFF_ALLMULTI flag is set on interface and IFF_PROMISC isn't, > emac_dev_mcast_set should only enable RX of multicasts and reset > MACHASH registers. > > It does this, but afterwards it either sets up multicast MACs > filtering or disables RX of multicasts and resets MACHASH registers > again, rendering IFF_ALLMULTI flag useless. > > This patch fixes emac_dev_mcast_set, so that multicast MACs filtering and > disabling of RX of multicasts are skipped when IFF_ALLMULTI flag is set. > > Tested with kernel 2.6.37. > > Signed-off-by: Mariusz Ceier Applied and queued up for -stable, thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] sched: Fix asymmetric scheduling for POWER7
Vaidyanathan Srinivasan wrote: > Asymmetric scheduling within a core is a scheduler loadbalancing > feature that is triggered when SD_ASYM_PACKING flag is set. The goal > for the load balancer is to move tasks to lower order idle SMT threads > within a core on a POWER7 system. > > In nohz_kick_needed(), we intend to check if our sched domain (core) > is completely busy or we have idle cpu. > > The following check for SD_ASYM_PACKING: > > (cpumask_first_and(nohz.idle_cpus_mask, sched_domain_span(sd)) < cpu) > > already covers the case of checking if the domain has an idle cpu, > because cpumask_first_and() will not yield any set bits if this domain > has no idle cpu. > > Hence, nr_busy check against group weight can be removed. > > Reported-by: Michael Neuling Tested-by: Michael Neuling Peter, I tested this only a brief while back but it turned out my test wasn't stringent enough and it was actually broken (in v3.9). This fixes it. Mikey > Signed-off-by: Vaidyanathan Srinivasan > Signed-off-by: Preeti U Murthy > --- > kernel/sched/fair.c |4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c > index 12f0eab..828ed97 100644 > --- a/kernel/sched/fair.c > +++ b/kernel/sched/fair.c > @@ -5821,8 +5821,8 @@ static inline int nohz_kick_needed(struct rq *rq, int > cpu) > goto need_kick_unlock; > } > > - if (sd->flags & SD_ASYM_PACKING && nr_busy != sg->group_weight > - && (cpumask_first_and(nohz.idle_cpus_mask, > + if (sd->flags & SD_ASYM_PACKING && > + (cpumask_first_and(nohz.idle_cpus_mask, > sched_domain_span(sd)) < cpu)) > goto need_kick_unlock; > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] [PATCH] ax88179_178a: Correct the RX error definition in RX header
From: fre...@asix.com.tw Date: Mon, 21 Oct 2013 14:37:40 +0800 > From: Freddy Xin > > Correct the definition of AX_RXHDR_CRC_ERR and > AX_RXHDR_DROP_ERR. They are BIT29 and BIT31 in pkt_hdr > seperately. > Add VID:DID for Samsung USB Ethernet Adapter. > > Signed-off-by: Freddy Xin Do not do two logically different things in one patch. Fix the register bit definitions in one patch, then add the new device IDs in another. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] extcon: palmas: Handle ID interrupt properly using USB_ID_INT_SRC
Hi George, On 10/15/2013 01:40 PM, George Cherian wrote: > Laxman/Chanwoo/Kishon, > > Any comments on this!! > Regards > -George > On 10/11/2013 12:18 AM, George Cherian wrote: >> Always cross check with the ID state and the source of interrupt. >> Also add a case in which ID Source is ID_GND but LATCH state is set >> wrongly. This uses the previous Link stat to determine the new state. >> >> Signed-off-by: George Cherian >> --- >> drivers/extcon/extcon-palmas.c | 15 --- >> 1 file changed, 12 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/extcon/extcon-palmas.c b/drivers/extcon/extcon-palmas.c >> index 6e83e9a..2aea4bc 100644 >> --- a/drivers/extcon/extcon-palmas.c >> +++ b/drivers/extcon/extcon-palmas.c >> @@ -78,20 +78,24 @@ static irqreturn_t palmas_vbus_irq_handler(int irq, void >> *_palmas_usb) >> static irqreturn_t palmas_id_irq_handler(int irq, void *_palmas_usb) >> { >> -unsigned int set; >> +unsigned int set, id_src; >> struct palmas_usb *palmas_usb = _palmas_usb; >> palmas_read(palmas_usb->palmas, PALMAS_USB_OTG_BASE, >> PALMAS_USB_ID_INT_LATCH_SET, ); >> +palmas_read(palmas_usb->palmas, PALMAS_USB_OTG_BASE, >> +PALMAS_USB_ID_INT_SRC, _src); >> -if (set & PALMAS_USB_ID_INT_SRC_ID_GND) { >> +if ((set & PALMAS_USB_ID_INT_SRC_ID_GND) && >> +(id_src & PALMAS_USB_ID_INT_SRC_ID_GND)) { >> palmas_write(palmas_usb->palmas, PALMAS_USB_OTG_BASE, >> PALMAS_USB_ID_INT_LATCH_CLR, >> PALMAS_USB_ID_INT_EN_HI_CLR_ID_GND); >> palmas_usb->linkstat = PALMAS_USB_STATE_ID; >> extcon_set_cable_state(_usb->edev, "USB-HOST", true); >> dev_info(palmas_usb->dev, "USB-HOST cable is attached\n"); >> -} else if (set & PALMAS_USB_ID_INT_SRC_ID_FLOAT) { >> +} else if ((set & PALMAS_USB_ID_INT_SRC_ID_FLOAT) && >> +(id_src & PALMAS_USB_ID_INT_SRC_ID_FLOAT)) { >> palmas_write(palmas_usb->palmas, PALMAS_USB_OTG_BASE, >> PALMAS_USB_ID_INT_LATCH_CLR, >> PALMAS_USB_ID_INT_EN_HI_CLR_ID_FLOAT); >> @@ -103,6 +107,11 @@ static irqreturn_t palmas_id_irq_handler(int irq, void >> *_palmas_usb) >> palmas_usb->linkstat = PALMAS_USB_STATE_DISCONNECT; >> extcon_set_cable_state(_usb->edev, "USB-HOST", false); >> dev_info(palmas_usb->dev, "USB-HOST cable is detached\n"); >> +} else if ((palmas_usb->linkstat == PALMAS_USB_STATE_DISCONNECT) && >> +(id_src & PALMAS_USB_ID_INT_SRC_ID_GND)) { >> +palmas_usb->linkstat = PALMAS_USB_STATE_ID; >> +extcon_set_cable_state(_usb->edev, "USB-HOST", true); >> +dev_info(palmas_usb->dev, " USB-HOST cable is attached\n"); >> } >> return IRQ_HANDLED; > > Applied it on next branch. Thanks. Chanwoo Choi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] cpufreq: intel_pstate: fix possible integer overflow
On Monday, October 21, 2013 03:43:51 PM Dirk Brandewie wrote: > On 10/21/2013 03:47 PM, Rafael J. Wysocki wrote: > > On Monday, October 21, 2013 08:56:22 AM Dirk Brandewie wrote: > >> On 10/19/2013 08:31 PM, Geyslan G. Bem wrote: > >>> The expression 'pstate << 8' is evaluated using 32-bit arithmetic while > >>> 'val' expects an expression of type u64. > >>> > >>> Signed-off-by: Geyslan G. Bem > >> Acked-by: Dirk Brandewie > > > > Actually, isn't (pstate << 8) guaranteed not to overflow? > > > > Yes, I was assuming this was caught by a static checking tool. What was caught by the tool was the fact that 1UL << 32 might overflow on 32-bit, so using BIT(32) wasn't correct. > I didn't see a downside to giving the compilier complete information. Well, in that case the function's argument should be u64 rather than int. Either you know that it won't overflow, in which case the explicit type casting doesn't change anything, or you are not sure, in which case it's better to use u64 as the original type anyway in my opinion. Thanks! -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 03/23] ima: provide support for arbitrary hash algorithms
From: Dmitry Kasatkin In preparation of supporting more hash algorithms with larger hash sizes needed for signature verification, this patch replaces the 20 byte sized digest, with a more flexible structure. The new structure includes the hash algorithm, digest size, and digest. Changelog: - recalculate filedata hash for the measurement list, if the signature hash digest size is greater than 20 bytes. - use generic HASH_ALGO_ - make ima_calc_file_hash static - scripts lindent and checkpatch fixes Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- crypto/asymmetric_keys/x509_parser.h | 2 -- crypto/asymmetric_keys/x509_public_key.c | 3 +- security/integrity/ima/Kconfig | 1 + security/integrity/ima/ima.h | 7 +++-- security/integrity/ima/ima_api.c | 32 +++-- security/integrity/ima/ima_appraise.c| 20 +++-- security/integrity/ima/ima_crypto.c | 49 ++-- security/integrity/ima/ima_main.c| 6 ++-- security/integrity/integrity.h | 15 -- 9 files changed, 100 insertions(+), 35 deletions(-) diff --git a/crypto/asymmetric_keys/x509_parser.h b/crypto/asymmetric_keys/x509_parser.h index 04c81bd..87d9cc2 100644 --- a/crypto/asymmetric_keys/x509_parser.h +++ b/crypto/asymmetric_keys/x509_parser.h @@ -21,8 +21,6 @@ struct x509_certificate { char*authority; /* Authority key fingerprint as hex */ struct tm valid_from; struct tm valid_to; - enum pkey_algo pkey_algo : 8; /* Public key algorithm */ - enum hash_algo sig_hash_algo : 8; /* Signature hash algorithm */ const void *tbs; /* Signed data */ unsignedtbs_size; /* Size of signed data */ unsignedraw_sig_size; /* Size of sigature */ diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c index 0a6bfad..f83300b 100644 --- a/crypto/asymmetric_keys/x509_public_key.c +++ b/crypto/asymmetric_keys/x509_public_key.c @@ -213,7 +213,8 @@ static int x509_key_preparse(struct key_preparsed_payload *prep) cert->valid_to.tm_year + 1900, cert->valid_to.tm_mon + 1, cert->valid_to.tm_mday, cert->valid_to.tm_hour, cert->valid_to.tm_min, cert->valid_to.tm_sec); - pr_devel("Cert Signature: %s\n", + pr_devel("Cert Signature: %s + %s\n", +pkey_algo_name[cert->sig.pkey_algo], hash_algo_name[cert->sig.pkey_hash_algo]); if (!cert->fingerprint) { diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig index 39196ab..e6628e7 100644 --- a/security/integrity/ima/Kconfig +++ b/security/integrity/ima/Kconfig @@ -9,6 +9,7 @@ config IMA select CRYPTO_HMAC select CRYPTO_MD5 select CRYPTO_SHA1 + select CRYPTO_HASH_INFO select TCG_TPM if HAS_IOMEM && !UML select TCG_TIS if TCG_TPM && X86 select TCG_IBMVTPM if TCG_TPM && PPC64 diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index b3dd616..eb86032 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -39,7 +39,7 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8 }; /* set during initialization */ extern int ima_initialized; extern int ima_used_chip; -extern char *ima_hash; +extern int ima_hash_algo; extern int ima_appraise; /* IMA inode template definition */ @@ -70,8 +70,9 @@ void ima_fs_cleanup(void); int ima_inode_alloc(struct inode *inode); int ima_add_template_entry(struct ima_template_entry *entry, int violation, const char *op, struct inode *inode); -int ima_calc_file_hash(struct file *file, char *digest); -int ima_calc_buffer_hash(const void *data, int len, char *digest); +int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash); +int ima_calc_buffer_hash(const void *data, int len, +struct ima_digest_data *hash); int ima_calc_boot_aggregate(char *digest); void ima_add_violation(struct inode *inode, const unsigned char *filename, const char *op, const char *cause); diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index 1c03e8f1..e531fe2 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -44,6 +44,7 @@ int ima_store_template(struct ima_template_entry *entry, const char *op = "add_template_measure"; const char *audit_cause = "hashing_error"; int result; + struct ima_digest_data hash; memset(entry->digest, 0, sizeof(entry->digest)); entry->template_name = IMA_TEMPLATE_NAME; @@ -51,14 +52,14 @@ int ima_store_template(struct ima_template_entry *entry, if (!violation) { result = ima_calc_buffer_hash(>template, -
[PATCH v2 01/23] crypto: provide single place for hash algo information
From: Dmitry Kasatkin This patch provides a single place for information about hash algorithms, such as hash sizes and kernel driver names, which will be used by IMA and the public key code. Changelog: - Fix sparse and checkpatch warnings - Move hash algo enums to uapi for userspace signing functions. Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- crypto/Kconfig | 3 +++ crypto/Makefile| 1 + crypto/hash_info.c | 56 ++ include/crypto/hash_info.h | 40 ++ include/uapi/linux/hash_info.h | 37 5 files changed, 137 insertions(+) create mode 100644 crypto/hash_info.c create mode 100644 include/crypto/hash_info.h create mode 100644 include/uapi/linux/hash_info.h diff --git a/crypto/Kconfig b/crypto/Kconfig index 69ce573..ba061b0 100644 --- a/crypto/Kconfig +++ b/crypto/Kconfig @@ -1386,6 +1386,9 @@ config CRYPTO_USER_API_SKCIPHER This option enables the user-spaces interface for symmetric key cipher algorithms. +config CRYPTO_HASH_INFO + bool + source "drivers/crypto/Kconfig" source crypto/asymmetric_keys/Kconfig diff --git a/crypto/Makefile b/crypto/Makefile index 80019ba..b3a7e80 100644 --- a/crypto/Makefile +++ b/crypto/Makefile @@ -104,3 +104,4 @@ obj-$(CONFIG_CRYPTO_USER_API_SKCIPHER) += algif_skcipher.o obj-$(CONFIG_XOR_BLOCKS) += xor.o obj-$(CONFIG_ASYNC_CORE) += async_tx/ obj-$(CONFIG_ASYMMETRIC_KEY_TYPE) += asymmetric_keys/ +obj-$(CONFIG_CRYPTO_HASH_INFO) += hash_info.o diff --git a/crypto/hash_info.c b/crypto/hash_info.c new file mode 100644 index 000..3e7ff46 --- /dev/null +++ b/crypto/hash_info.c @@ -0,0 +1,56 @@ +/* + * Hash Info: Hash algorithms information + * + * Copyright (c) 2013 Dmitry Kasatkin + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 of the License, or (at your option) + * any later version. + * + */ + +#include +#include + +const char *const hash_algo_name[HASH_ALGO__LAST] = { + [HASH_ALGO_MD4] = "md4", + [HASH_ALGO_MD5] = "md5", + [HASH_ALGO_SHA1]= "sha1", + [HASH_ALGO_RIPE_MD_160] = "rmd160", + [HASH_ALGO_SHA256] = "sha256", + [HASH_ALGO_SHA384] = "sha384", + [HASH_ALGO_SHA512] = "sha512", + [HASH_ALGO_SHA224] = "sha224", + [HASH_ALGO_RIPE_MD_128] = "rmd128", + [HASH_ALGO_RIPE_MD_256] = "rmd256", + [HASH_ALGO_RIPE_MD_320] = "rmd320", + [HASH_ALGO_WP_256] = "wp256", + [HASH_ALGO_WP_384] = "wp384", + [HASH_ALGO_WP_512] = "wp512", + [HASH_ALGO_TGR_128] = "tgr128", + [HASH_ALGO_TGR_160] = "tgr160", + [HASH_ALGO_TGR_192] = "tgr192", +}; +EXPORT_SYMBOL_GPL(hash_algo_name); + +const int hash_digest_size[HASH_ALGO__LAST] = { + [HASH_ALGO_MD4] = MD5_DIGEST_SIZE, + [HASH_ALGO_MD5] = MD5_DIGEST_SIZE, + [HASH_ALGO_SHA1]= SHA1_DIGEST_SIZE, + [HASH_ALGO_RIPE_MD_160] = RMD160_DIGEST_SIZE, + [HASH_ALGO_SHA256] = SHA256_DIGEST_SIZE, + [HASH_ALGO_SHA384] = SHA384_DIGEST_SIZE, + [HASH_ALGO_SHA512] = SHA512_DIGEST_SIZE, + [HASH_ALGO_SHA224] = SHA224_DIGEST_SIZE, + [HASH_ALGO_RIPE_MD_128] = RMD128_DIGEST_SIZE, + [HASH_ALGO_RIPE_MD_256] = RMD256_DIGEST_SIZE, + [HASH_ALGO_RIPE_MD_320] = RMD320_DIGEST_SIZE, + [HASH_ALGO_WP_256] = WP256_DIGEST_SIZE, + [HASH_ALGO_WP_384] = WP384_DIGEST_SIZE, + [HASH_ALGO_WP_512] = WP512_DIGEST_SIZE, + [HASH_ALGO_TGR_128] = TGR128_DIGEST_SIZE, + [HASH_ALGO_TGR_160] = TGR160_DIGEST_SIZE, + [HASH_ALGO_TGR_192] = TGR192_DIGEST_SIZE, +}; +EXPORT_SYMBOL_GPL(hash_digest_size); diff --git a/include/crypto/hash_info.h b/include/crypto/hash_info.h new file mode 100644 index 000..e1e5a3e --- /dev/null +++ b/include/crypto/hash_info.h @@ -0,0 +1,40 @@ +/* + * Hash Info: Hash algorithms information + * + * Copyright (c) 2013 Dmitry Kasatkin + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 of the License, or (at your option) + * any later version. + * + */ + +#ifndef _CRYPTO_HASH_INFO_H +#define _CRYPTO_HASH_INFO_H + +#include +#include + +#include + +/* not defined in include/crypto/ */ +#define RMD128_DIGEST_SIZE 16 +#define RMD160_DIGEST_SIZE 20 +#define RMD256_DIGEST_SIZE 32 +#define RMD320_DIGEST_SIZE 40 + +/* not defined in include/crypto/ */ +#define WP512_DIGEST_SIZE 64 +#define WP384_DIGEST_SIZE 48 +#define WP256_DIGEST_SIZE 32 + +/* not defined in include/crypto/ */ +#define
[PATCH v2 05/23] ima: pass full xattr with the signature
From: Dmitry Kasatkin For possibility to use xattr type for new signature formats, pass full xattr to the signature verification function. Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- security/integrity/digsig.c | 5 +++-- security/integrity/evm/evm_main.c | 4 ++-- security/integrity/ima/ima_appraise.c | 2 +- security/integrity/integrity.h| 1 + 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c index 198e609..b4af4eb 100644 --- a/security/integrity/digsig.c +++ b/security/integrity/digsig.c @@ -44,9 +44,10 @@ int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen, } } - switch (sig[0]) { + switch (sig[1]) { case 1: - return digsig_verify(keyring[id], sig, siglen, + /* v1 API expect signature without xattr type */ + return digsig_verify(keyring[id], sig + 1, siglen - 1, digest, digestlen); case 2: return asymmetric_verify(keyring[id], sig, siglen, diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index af9b685..336b3dd 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -123,7 +123,7 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry, goto out; } - xattr_len = rc - 1; + xattr_len = rc; /* check value type */ switch (xattr_data->type) { @@ -143,7 +143,7 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry, if (rc) break; rc = integrity_digsig_verify(INTEGRITY_KEYRING_EVM, - xattr_data->digest, xattr_len, + (const char *)xattr_data, xattr_len, calc.digest, sizeof(calc.digest)); if (!rc) { /* we probably want to replace rsa with hmac here */ diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index 00708a3..e1865a6 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -205,7 +205,7 @@ int ima_appraise_measurement(int func, struct integrity_iint_cache *iint, case EVM_IMA_XATTR_DIGSIG: iint->flags |= IMA_DIGSIG; rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA, -xattr_value->digest, rc - 1, +(const char *)xattr_value, rc, iint->ima_hash.digest, iint->ima_hash.length); if (rc == -EOPNOTSUPP) { diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h index ea23189..aead6b2 100644 --- a/security/integrity/integrity.h +++ b/security/integrity/integrity.h @@ -74,6 +74,7 @@ struct ima_digest_data { * signature format v2 - for using with asymmetric keys */ struct signature_v2_hdr { + uint8_t type; /* xattr type */ uint8_t version;/* signature format version */ uint8_t hash_algo; /* Digest algorithm [enum pkey_hash_algo] */ uint32_t keyid; /* IMA key identifier - not X509/PGP specific */ -- 1.8.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 06/23] ima: use dynamically allocated hash storage
From: Dmitry Kasatkin For each inode in the IMA policy, an iint is allocated. To support larger hash digests, the iint digest size changed from 20 bytes to the maximum supported hash digest size. Instead of allocating the maximum size, which most likely is not needed, this patch dynamically allocates the needed hash storage. Changelog: - fix krealloc bug Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- security/integrity/iint.c | 2 ++ security/integrity/ima/ima_api.c | 57 +++ security/integrity/ima/ima_appraise.c | 16 +- security/integrity/integrity.h| 4 +-- 4 files changed, 49 insertions(+), 30 deletions(-) diff --git a/security/integrity/iint.c b/security/integrity/iint.c index 74522db..c49d3f1 100644 --- a/security/integrity/iint.c +++ b/security/integrity/iint.c @@ -70,6 +70,8 @@ struct integrity_iint_cache *integrity_iint_find(struct inode *inode) static void iint_free(struct integrity_iint_cache *iint) { + kfree(iint->ima_hash); + iint->ima_hash = NULL; iint->version = 0; iint->flags = 0UL; iint->ima_file_status = INTEGRITY_UNKNOWN; diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index 1dba98e..5a7942e 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -44,7 +44,10 @@ int ima_store_template(struct ima_template_entry *entry, const char *op = "add_template_measure"; const char *audit_cause = "hashing_error"; int result; - struct ima_digest_data hash; + struct { + struct ima_digest_data hdr; + char digest[IMA_MAX_DIGEST_SIZE]; + } hash; memset(entry->digest, 0, sizeof(entry->digest)); entry->template_name = IMA_TEMPLATE_NAME; @@ -52,14 +55,14 @@ int ima_store_template(struct ima_template_entry *entry, if (!violation) { result = ima_calc_buffer_hash(>template, - entry->template_len, ); + entry->template_len, ); if (result < 0) { integrity_audit_msg(AUDIT_INTEGRITY_PCR, inode, entry->template_name, op, audit_cause, result, 0); return result; } - memcpy(entry->digest, hash.digest, hash.length); + memcpy(entry->digest, hash.hdr.digest, hash.hdr.length); } result = ima_add_template_entry(entry, violation, op, inode); return result; @@ -146,6 +149,10 @@ int ima_collect_measurement(struct integrity_iint_cache *iint, struct inode *inode = file_inode(file); const char *filename = file->f_dentry->d_name.name; int result = 0; + struct { + struct ima_digest_data hdr; + char digest[IMA_MAX_DIGEST_SIZE]; + } hash; if (xattr_value) *xattr_len = ima_read_xattr(file->f_dentry, xattr_value); @@ -154,16 +161,23 @@ int ima_collect_measurement(struct integrity_iint_cache *iint, u64 i_version = file_inode(file)->i_version; /* use default hash algorithm */ - iint->ima_hash.algo = ima_hash_algo; + hash.hdr.algo = ima_hash_algo; if (xattr_value) - ima_get_hash_algo(*xattr_value, *xattr_len, - >ima_hash); + ima_get_hash_algo(*xattr_value, *xattr_len, ); - result = ima_calc_file_hash(file, >ima_hash); + result = ima_calc_file_hash(file, ); if (!result) { - iint->version = i_version; - iint->flags |= IMA_COLLECTED; + int length = sizeof(hash.hdr) + hash.hdr.length; + void *tmpbuf = krealloc(iint->ima_hash, length, + GFP_NOFS); + if (tmpbuf) { + iint->ima_hash = tmpbuf; + memcpy(iint->ima_hash, , length); + iint->version = i_version; + iint->flags |= IMA_COLLECTED; + } else + result = -ENOMEM; } } if (result) @@ -208,21 +222,24 @@ void ima_store_measurement(struct integrity_iint_cache *iint, return; } memset(>template, 0, sizeof(entry->template)); - if (iint->ima_hash.algo != ima_hash_algo) { - struct ima_digest_data hash; + if (iint->ima_hash->algo != ima_hash_algo) { + struct { + struct ima_digest_data hdr; + char digest[IMA_MAX_DIGEST_SIZE]; +
[PATCH v2 04/23] ima: read and use signature hash algorithm
From: Dmitry Kasatkin All files on the filesystem, currently, are hashed using the same hash algorithm. In preparation for files from different packages being signed using different hash algorithms, this patch adds support for reading the signature hash algorithm from the 'security.ima' extended attribute and calculates the appropriate file data hash based on it. Changelog: - fix scripts Lindent and checkpatch msgs - Mimi - fix md5 support for older version, which occupied 20 bytes in the xattr, not the expected 16 bytes. Fix the comparison to compare only the first 16 bytes. Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- security/integrity/digsig_asymmetric.c | 11 - security/integrity/ima/ima.h | 29 +++--- security/integrity/ima/ima_api.c | 12 - security/integrity/ima/ima_appraise.c | 45 -- security/integrity/ima/ima_main.c | 11 +++-- security/integrity/integrity.h | 11 + 6 files changed, 94 insertions(+), 25 deletions(-) diff --git a/security/integrity/digsig_asymmetric.c b/security/integrity/digsig_asymmetric.c index b475466..9eae480 100644 --- a/security/integrity/digsig_asymmetric.c +++ b/security/integrity/digsig_asymmetric.c @@ -20,17 +20,6 @@ #include "integrity.h" /* - * signature format v2 - for using with asymmetric keys - */ -struct signature_v2_hdr { - uint8_t version;/* signature format version */ - uint8_t hash_algo; /* Digest algorithm [enum pkey_hash_algo] */ - uint32_t keyid; /* IMA key identifier - not X509/PGP specific*/ - uint16_t sig_size; /* signature size */ - uint8_t sig[0]; /* signature payload */ -} __packed; - -/* * Request an asymmetric key. */ static struct key *request_asymmetric_key(struct key *keyring, uint32_t keyid) diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index eb86032..efcdef2 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -99,7 +99,9 @@ static inline unsigned long ima_hash_key(u8 *digest) int ima_get_action(struct inode *inode, int mask, int function); int ima_must_measure(struct inode *inode, int mask, int function); int ima_collect_measurement(struct integrity_iint_cache *iint, - struct file *file); + struct file *file, + struct evm_ima_xattr_data **xattr_value, + int *xattr_len); void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file, const unsigned char *filename); void ima_audit_measurement(struct integrity_iint_cache *iint, @@ -132,17 +134,25 @@ void ima_delete_rules(void); #ifdef CONFIG_IMA_APPRAISE int ima_appraise_measurement(int func, struct integrity_iint_cache *iint, -struct file *file, const unsigned char *filename); +struct file *file, const unsigned char *filename, +struct evm_ima_xattr_data *xattr_value, +int xattr_len); int ima_must_appraise(struct inode *inode, int mask, enum ima_hooks func); void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file); enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint, int func); +void ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value, int xattr_len, + struct ima_digest_data *hash); +int ima_read_xattr(struct dentry *dentry, + struct evm_ima_xattr_data **xattr_value); #else static inline int ima_appraise_measurement(int func, struct integrity_iint_cache *iint, struct file *file, - const unsigned char *filename) + const unsigned char *filename, + struct evm_ima_xattr_data *xattr_value, + int xattr_len) { return INTEGRITY_UNKNOWN; } @@ -163,6 +173,19 @@ static inline enum integrity_status ima_get_cache_status(struct integrity_iint_c { return INTEGRITY_UNKNOWN; } + +static inline void ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value, +int xattr_len, +struct ima_digest_data *hash) +{ +} + +static inline int ima_read_xattr(struct dentry *dentry, +struct evm_ima_xattr_data **xattr_value) +{ + return 0; +} + #endif /* LSM based policy rules require audit */ diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index e531fe2..1dba98e 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -139,17
[PATCH v2 07/23] ima: differentiate between template hash and file data hash sizes
The TPM v1.2 limits the template hash size to 20 bytes. This patch differentiates between the template hash size, as defined in the ima_template_entry, and the file data hash size, as defined in the ima_template_data. Subsequent patches add support for different file data hash algorithms. Change log: - hash digest definition in ima_store_template() should be TPM_DIGEST_SIZE Signed-off-by: Mimi Zohar --- security/integrity/ima/ima.h| 2 +- security/integrity/ima/ima_api.c| 2 +- security/integrity/ima/ima_crypto.c | 4 ++-- security/integrity/ima/ima_fs.c | 10 +- security/integrity/ima/ima_init.c | 2 +- security/integrity/ima/ima_queue.c | 4 ++-- 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index efcdef2..52393ed 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -49,7 +49,7 @@ struct ima_template_data { }; struct ima_template_entry { - u8 digest[IMA_DIGEST_SIZE]; /* sha1 or md5 measurement hash */ + u8 digest[TPM_DIGEST_SIZE]; /* sha1 or md5 measurement hash */ const char *template_name; int template_len; struct ima_template_data template; diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index 5a7942e..2cc5dcc 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -46,7 +46,7 @@ int ima_store_template(struct ima_template_entry *entry, int result; struct { struct ima_digest_data hdr; - char digest[IMA_MAX_DIGEST_SIZE]; + char digest[TPM_DIGEST_SIZE]; } hash; memset(entry->digest, 0, sizeof(entry->digest)); diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c index 2fd1786..872c669 100644 --- a/security/integrity/ima/ima_crypto.c +++ b/security/integrity/ima/ima_crypto.c @@ -155,7 +155,7 @@ static void __init ima_pcrread(int idx, u8 *pcr) */ int __init ima_calc_boot_aggregate(char *digest) { - u8 pcr_i[IMA_DIGEST_SIZE]; + u8 pcr_i[TPM_DIGEST_SIZE]; int rc, i; struct { struct shash_desc shash; @@ -173,7 +173,7 @@ int __init ima_calc_boot_aggregate(char *digest) for (i = TPM_PCR0; i < TPM_PCR8; i++) { ima_pcrread(i, pcr_i); /* now accumulate with current aggregate */ - rc = crypto_shash_update(, pcr_i, IMA_DIGEST_SIZE); + rc = crypto_shash_update(, pcr_i, TPM_DIGEST_SIZE); } if (!rc) crypto_shash_final(, digest); diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c index 5f0fd11..c35cfb5 100644 --- a/security/integrity/ima/ima_fs.c +++ b/security/integrity/ima/ima_fs.c @@ -133,7 +133,7 @@ static int ima_measurements_show(struct seq_file *m, void *v) ima_putc(m, , sizeof pcr); /* 2nd: template digest */ - ima_putc(m, e->digest, IMA_DIGEST_SIZE); + ima_putc(m, e->digest, TPM_DIGEST_SIZE); /* 3rd: template name size */ namelen = strlen(e->template_name); @@ -167,11 +167,11 @@ static const struct file_operations ima_measurements_ops = { .release = seq_release, }; -static void ima_print_digest(struct seq_file *m, u8 *digest) +static void ima_print_digest(struct seq_file *m, u8 *digest, int size) { int i; - for (i = 0; i < IMA_DIGEST_SIZE; i++) + for (i = 0; i < size; i++) seq_printf(m, "%02x", *(digest + i)); } @@ -182,7 +182,7 @@ void ima_template_show(struct seq_file *m, void *e, enum ima_show_type show) switch (show) { case IMA_SHOW_ASCII: - ima_print_digest(m, entry->digest); + ima_print_digest(m, entry->digest, IMA_DIGEST_SIZE); seq_printf(m, " %s\n", entry->file_name); break; case IMA_SHOW_BINARY: @@ -212,7 +212,7 @@ static int ima_ascii_measurements_show(struct seq_file *m, void *v) seq_printf(m, "%2d ", CONFIG_IMA_MEASURE_PCR_IDX); /* 2nd: SHA1 template hash */ - ima_print_digest(m, e->digest); + ima_print_digest(m, e->digest, TPM_DIGEST_SIZE); /* 3th: template name */ seq_printf(m, " %s ", e->template_name); diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c index 162ea72..9d0243c 100644 --- a/security/integrity/ima/ima_init.c +++ b/security/integrity/ima/ima_init.c @@ -74,7 +74,7 @@ err_out: int __init ima_init(void) { - u8 pcr_i[IMA_DIGEST_SIZE]; + u8 pcr_i[TPM_DIGEST_SIZE]; int rc; ima_used_chip = 0; diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c index ff63fe0..e63ff33 100644 --- a/security/integrity/ima/ima_queue.c +++ b/security/integrity/ima/ima_queue.c @@ -50,7 +50,7 @@ static struct ima_queue_entry
[PATCH v2 12/23] ima: pass the filename argument up to ima_add_template_entry()
From: Roberto Sassu Pass the filename argument to ima_add_template_entry() in order to eliminate a dependency on template specific data (third argument of integrity_audit_msg). This change is required because, with the new template management mechanism, the generation of a new measurement entry will be performed by new specific functions (introduced in next patches) and the current IMA code will not be aware anymore of how data is stored in the entry payload. Signed-off-by: Roberto Sassu Signed-off-by: Mimi Zohar --- security/integrity/ima/ima.h | 5 +++-- security/integrity/ima/ima_api.c | 9 + security/integrity/ima/ima_init.c | 3 ++- security/integrity/ima/ima_queue.c | 6 +++--- 4 files changed, 13 insertions(+), 10 deletions(-) diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index d7bec6f..27d2ffb 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -69,7 +69,8 @@ int ima_fs_init(void); void ima_fs_cleanup(void); int ima_inode_alloc(struct inode *inode); int ima_add_template_entry(struct ima_template_entry *entry, int violation, - const char *op, struct inode *inode); + const char *op, struct inode *inode, + const unsigned char *filename); int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash); int ima_calc_buffer_hash(const void *data, int len, struct ima_digest_data *hash); @@ -107,7 +108,7 @@ void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file, void ima_audit_measurement(struct integrity_iint_cache *iint, const unsigned char *filename); int ima_store_template(struct ima_template_entry *entry, int violation, - struct inode *inode); + struct inode *inode, const unsigned char *filename); void ima_template_show(struct seq_file *m, void *e, enum ima_show_type show); const char *ima_d_path(struct path *path, char **pathbuf); diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index 98160a3..a0fe504 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -40,7 +40,8 @@ static const char *IMA_TEMPLATE_NAME = "ima"; * Returns 0 on success, error code otherwise */ int ima_store_template(struct ima_template_entry *entry, - int violation, struct inode *inode) + int violation, struct inode *inode, + const unsigned char *filename) { const char *op = "add_template_measure"; const char *audit_cause = "hashing_error"; @@ -67,7 +68,7 @@ int ima_store_template(struct ima_template_entry *entry, } memcpy(entry->digest, hash.hdr.digest, hash.hdr.length); } - result = ima_add_template_entry(entry, violation, op, inode); + result = ima_add_template_entry(entry, violation, op, inode, filename); return result; } @@ -96,7 +97,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename, } memset(>template, 0, sizeof(entry->template)); strncpy(entry->template.file_name, filename, IMA_EVENT_NAME_LEN_MAX); - result = ima_store_template(entry, violation, inode); + result = ima_store_template(entry, violation, inode, filename); if (result < 0) kfree(entry); err_out: @@ -248,7 +249,7 @@ void ima_store_measurement(struct integrity_iint_cache *iint, (strlen(filename) > IMA_EVENT_NAME_LEN_MAX) ? file->f_dentry->d_name.name : filename); - result = ima_store_template(entry, violation, inode); + result = ima_store_template(entry, violation, inode, filename); if (!result || result == -EEXIST) iint->flags |= IMA_MEASURED; if (result < 0) diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c index 77cd500..d42fac3 100644 --- a/security/integrity/ima/ima_init.c +++ b/security/integrity/ima/ima_init.c @@ -71,7 +71,8 @@ static void __init ima_add_boot_aggregate(void) memcpy(entry->template.digest, hash.hdr.digest, hash.hdr.length); } - result = ima_store_template(entry, violation, NULL); + result = ima_store_template(entry, violation, NULL, + boot_aggregate_name); if (result < 0) kfree(entry); return; diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c index e63ff33..d85e997 100644 --- a/security/integrity/ima/ima_queue.c +++ b/security/integrity/ima/ima_queue.c @@ -104,7 +104,8 @@ static int ima_pcr_extend(const u8 *hash) * and extend the pcr. */ int ima_add_template_entry(struct ima_template_entry *entry, int violation, - const char *op, struct inode *inode) +
[PATCH v2 13/23] ima: define new function ima_alloc_init_template() to API
From: Roberto Sassu Instead of allocating and initializing the template entry from multiple places (eg. boot aggregate, violation, and regular measurements), this patch defines a new function called ima_alloc_init_template(). The new function allocates and initializes the measurement entry with the inode digest and the filename. In respect to the current behavior, it truncates the file name passed in the 'filename' argument if the latter's size is greater than 255 bytes and the passed file descriptor is NULL. Changelog: - initialize 'hash' variable for non TPM case - Mimi - conform to expectation for 'iint' to be defined as a pointer. - Mimi - add missing 'file' dependency for recalculating file hash. - Mimi Signed-off-by: Roberto Sassu Signed-off-by: Mimi Zohar --- security/integrity/ima/ima.h | 3 ++ security/integrity/ima/ima_api.c | 88 ++- security/integrity/ima/ima_init.c | 24 ++- 3 files changed, 76 insertions(+), 39 deletions(-) diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 27d2ffb..da03d33 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -107,6 +107,9 @@ void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file, const unsigned char *filename); void ima_audit_measurement(struct integrity_iint_cache *iint, const unsigned char *filename); +int ima_alloc_init_template(struct integrity_iint_cache *iint, + struct file *file, const unsigned char *filename, + struct ima_template_entry **entry); int ima_store_template(struct ima_template_entry *entry, int violation, struct inode *inode, const unsigned char *filename); void ima_template_show(struct seq_file *m, void *e, enum ima_show_type show); diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index a0fe504..29dd43d 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -24,6 +24,62 @@ static const char *IMA_TEMPLATE_NAME = "ima"; /* + * ima_alloc_init_template - create and initialize a new template entry + */ +int ima_alloc_init_template(struct integrity_iint_cache *iint, + struct file *file, const unsigned char *filename, + struct ima_template_entry **entry) +{ + struct ima_template_entry *e; + int result = 0; + + e = kzalloc(sizeof(**entry), GFP_NOFS); + if (!e) + return -ENOMEM; + + memset(&(e)->template, 0, sizeof(e->template)); + if (!iint) /* IMA measurement violation entry */ + goto out; + + if (iint->ima_hash->algo != ima_hash_algo) { + struct inode *inode; + struct { + struct ima_digest_data hdr; + char digest[IMA_MAX_DIGEST_SIZE]; + } hash; + + if (!file) { + result = -EINVAL; + goto out_free; + } + + inode = file_inode(file); + hash.hdr.algo = ima_hash_algo; + hash.hdr.length = SHA1_DIGEST_SIZE; + result = ima_calc_file_hash(file, ); + if (result) { + integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode, + filename, "collect_data", + "failed", result, 0); + goto out_free; + } else + memcpy(e->template.digest, hash.hdr.digest, + hash.hdr.length); + } else + memcpy(e->template.digest, iint->ima_hash->digest, + iint->ima_hash->length); +out: + strcpy(e->template.file_name, + (strlen(filename) > IMA_EVENT_NAME_LEN_MAX && file != NULL) ? + file->f_dentry->d_name.name : filename); + *entry = e; + return 0; +out_free: + kfree(e); + return result; +} + +/* * ima_store_template - store ima template measurements * * Calculate the hash of a template entry, add the template entry @@ -90,13 +146,11 @@ void ima_add_violation(struct file *file, const unsigned char *filename, /* can overflow, only indicator */ atomic_long_inc(_htable.violations); - entry = kmalloc(sizeof(*entry), GFP_KERNEL); - if (!entry) { + result = ima_alloc_init_template(NULL, file, filename, ); + if (result < 0) { result = -ENOMEM; goto err_out; } - memset(>template, 0, sizeof(entry->template)); - strncpy(entry->template.file_name, filename, IMA_EVENT_NAME_LEN_MAX); result = ima_store_template(entry, violation, inode, filename); if (result < 0) kfree(entry); @@ -220,34 +274,12
[PATCH v2 02/23] keys: change asymmetric keys to use common hash definitions
From: Dmitry Kasatkin This patch makes use of the newly defined common hash algorithm info, replacing, for example, PKEY_HASH with HASH_ALGO. Changelog: - Lindent fixes - Mimi Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- crypto/asymmetric_keys/Kconfig| 1 + crypto/asymmetric_keys/public_key.c | 12 crypto/asymmetric_keys/rsa.c | 14 +++--- crypto/asymmetric_keys/x509_cert_parser.c | 12 ++-- crypto/asymmetric_keys/x509_parser.h | 2 ++ crypto/asymmetric_keys/x509_public_key.c | 9 - include/crypto/public_key.h | 18 -- kernel/module_signing.c | 8 8 files changed, 28 insertions(+), 48 deletions(-) diff --git a/crypto/asymmetric_keys/Kconfig b/crypto/asymmetric_keys/Kconfig index 862b01f..82e7d6b 100644 --- a/crypto/asymmetric_keys/Kconfig +++ b/crypto/asymmetric_keys/Kconfig @@ -13,6 +13,7 @@ config ASYMMETRIC_PUBLIC_KEY_SUBTYPE tristate "Asymmetric public-key crypto algorithm subtype" select MPILIB select PUBLIC_KEY_ALGO_RSA + select CRYPTO_HASH_INFO help This option provides support for asymmetric public key type handling. If signature generation and/or verification are to be used, diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c index 49ac8d8..97eb001 100644 --- a/crypto/asymmetric_keys/public_key.c +++ b/crypto/asymmetric_keys/public_key.c @@ -36,18 +36,6 @@ const struct public_key_algorithm *pkey_algo[PKEY_ALGO__LAST] = { }; EXPORT_SYMBOL_GPL(pkey_algo); -const char *const pkey_hash_algo_name[PKEY_HASH__LAST] = { - [PKEY_HASH_MD4] = "md4", - [PKEY_HASH_MD5] = "md5", - [PKEY_HASH_SHA1]= "sha1", - [PKEY_HASH_RIPE_MD_160] = "rmd160", - [PKEY_HASH_SHA256] = "sha256", - [PKEY_HASH_SHA384] = "sha384", - [PKEY_HASH_SHA512] = "sha512", - [PKEY_HASH_SHA224] = "sha224", -}; -EXPORT_SYMBOL_GPL(pkey_hash_algo_name); - const char *const pkey_id_type_name[PKEY_ID_TYPE__LAST] = { [PKEY_ID_PGP] = "PGP", [PKEY_ID_X509] = "X509", diff --git a/crypto/asymmetric_keys/rsa.c b/crypto/asymmetric_keys/rsa.c index 4a6a069..90a17f5 100644 --- a/crypto/asymmetric_keys/rsa.c +++ b/crypto/asymmetric_keys/rsa.c @@ -73,13 +73,13 @@ static const struct { size_t size; } RSA_ASN1_templates[PKEY_HASH__LAST] = { #define _(X) { RSA_digest_info_##X, sizeof(RSA_digest_info_##X) } - [PKEY_HASH_MD5] = _(MD5), - [PKEY_HASH_SHA1]= _(SHA1), - [PKEY_HASH_RIPE_MD_160] = _(RIPE_MD_160), - [PKEY_HASH_SHA256] = _(SHA256), - [PKEY_HASH_SHA384] = _(SHA384), - [PKEY_HASH_SHA512] = _(SHA512), - [PKEY_HASH_SHA224] = _(SHA224), + [HASH_ALGO_MD5] = _(MD5), + [HASH_ALGO_SHA1]= _(SHA1), + [HASH_ALGO_RIPE_MD_160] = _(RIPE_MD_160), + [HASH_ALGO_SHA256] = _(SHA256), + [HASH_ALGO_SHA384] = _(SHA384), + [HASH_ALGO_SHA512] = _(SHA512), + [HASH_ALGO_SHA224] = _(SHA224), #undef _ }; diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c index 144201c..2989316 100644 --- a/crypto/asymmetric_keys/x509_cert_parser.c +++ b/crypto/asymmetric_keys/x509_cert_parser.c @@ -154,32 +154,32 @@ int x509_note_pkey_algo(void *context, size_t hdrlen, return -ENOPKG; /* Unsupported combination */ case OID_md4WithRSAEncryption: - ctx->cert->sig.pkey_hash_algo = PKEY_HASH_MD5; + ctx->cert->sig.pkey_hash_algo = HASH_ALGO_MD5; ctx->cert->sig.pkey_algo = PKEY_ALGO_RSA; break; case OID_sha1WithRSAEncryption: - ctx->cert->sig.pkey_hash_algo = PKEY_HASH_SHA1; + ctx->cert->sig.pkey_hash_algo = HASH_ALGO_SHA1; ctx->cert->sig.pkey_algo = PKEY_ALGO_RSA; break; case OID_sha256WithRSAEncryption: - ctx->cert->sig.pkey_hash_algo = PKEY_HASH_SHA256; + ctx->cert->sig.pkey_hash_algo = HASH_ALGO_SHA256; ctx->cert->sig.pkey_algo = PKEY_ALGO_RSA; break; case OID_sha384WithRSAEncryption: - ctx->cert->sig.pkey_hash_algo = PKEY_HASH_SHA384; + ctx->cert->sig.pkey_hash_algo = HASH_ALGO_SHA384; ctx->cert->sig.pkey_algo = PKEY_ALGO_RSA; break; case OID_sha512WithRSAEncryption: - ctx->cert->sig.pkey_hash_algo = PKEY_HASH_SHA512; + ctx->cert->sig.pkey_hash_algo = HASH_ALGO_SHA512; ctx->cert->sig.pkey_algo = PKEY_ALGO_RSA; break; case OID_sha224WithRSAEncryption: - ctx->cert->sig.pkey_hash_algo = PKEY_HASH_SHA224; +
[PATCH v2 11/23] ima: pass the file descriptor to ima_add_violation()
From: Roberto Sassu Pass the file descriptor instead of the inode to ima_add_violation(), to make the latter consistent with ima_store_measurement() in preparation for the new template architecture. Signed-off-by: Roberto Sassu Signed-off-by: Mimi Zohar --- security/integrity/ima/ima.h | 2 +- security/integrity/ima/ima_api.c | 3 ++- security/integrity/ima/ima_main.c | 5 ++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index e0e1cde..d7bec6f 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -74,7 +74,7 @@ int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash); int ima_calc_buffer_hash(const void *data, int len, struct ima_digest_data *hash); int __init ima_calc_boot_aggregate(struct ima_digest_data *hash); -void ima_add_violation(struct inode *inode, const unsigned char *filename, +void ima_add_violation(struct file *file, const unsigned char *filename, const char *op, const char *cause); int ima_init_crypto(void); diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index bc1d128..98160a3 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -78,10 +78,11 @@ int ima_store_template(struct ima_template_entry *entry, * By extending the PCR with 0xFF's instead of with zeroes, the PCR * value is invalidated. */ -void ima_add_violation(struct inode *inode, const unsigned char *filename, +void ima_add_violation(struct file *file, const unsigned char *filename, const char *op, const char *cause) { struct ima_template_entry *entry; + struct inode *inode = file->f_dentry->d_inode; int violation = 1; int result; diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 95b5df2..5e8b1f7 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -94,10 +94,9 @@ out: pathname = dentry->d_name.name; if (send_tomtou) - ima_add_violation(inode, pathname, - "invalid_pcr", "ToMToU"); + ima_add_violation(file, pathname, "invalid_pcr", "ToMToU"); if (send_writers) - ima_add_violation(inode, pathname, + ima_add_violation(file, pathname, "invalid_pcr", "open_writers"); kfree(pathbuf); } -- 1.8.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 15/23] ima: define template fields library and new helpers
From: Roberto Sassu This patch defines a library containing two initial template fields, inode digest (d) and file name (n), the 'ima' template descriptor, whose format is 'd|n', and two helper functions, ima_write_template_field_data() and ima_show_template_field_data(). Changelog: - replace ima_eventname_init() parameter NULL checking with BUG_ON. (suggested by Mimi) - include "new template fields for inode digest (d) and file name (n)" definitions to fix a compiler warning. - Mimi - unnecessary to prefix static function names with 'ima_'. remove prefix to resolve Lindent formatting changes. - Mimi - abbreviated/removed inline comments - Mimi - always send the template field length - Mimi Signed-off-by: Roberto Sassu Signed-off-by: Mimi Zohar --- security/integrity/ima/Makefile | 2 +- security/integrity/ima/ima.h | 5 + security/integrity/ima/ima_fs.c | 4 +- security/integrity/ima/ima_template.c | 15 ++- security/integrity/ima/ima_template_lib.c | 193 ++ security/integrity/ima/ima_template_lib.h | 31 + 6 files changed, 242 insertions(+), 8 deletions(-) create mode 100644 security/integrity/ima/ima_template_lib.c create mode 100644 security/integrity/ima/ima_template_lib.h diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile index 7fe4ae3..d79263d 100644 --- a/security/integrity/ima/Makefile +++ b/security/integrity/ima/Makefile @@ -6,5 +6,5 @@ obj-$(CONFIG_IMA) += ima.o ima-y := ima_fs.o ima_queue.o ima_init.o ima_main.o ima_crypto.o ima_api.o \ -ima_policy.o ima_template.o +ima_policy.o ima_template.o ima_template_lib.o ima-$(CONFIG_IMA_APPRAISE) += ima_appraise.o diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index c85718f..e1f081d 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -39,6 +39,9 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8 }; #define IMA_TEMPLATE_FIELD_ID_MAX_LEN 16 #define IMA_TEMPLATE_NUM_FIELDS_MAX15 +#define IMA_TEMPLATE_IMA_NAME "ima" +#define IMA_TEMPLATE_IMA_FMT "d|n" + /* set during initialization */ extern int ima_initialized; extern int ima_used_chip; @@ -105,6 +108,8 @@ int __init ima_calc_boot_aggregate(struct ima_digest_data *hash); void ima_add_violation(struct file *file, const unsigned char *filename, const char *op, const char *cause); int ima_init_crypto(void); +void ima_putc(struct seq_file *m, void *data, int datalen); +void ima_print_digest(struct seq_file *m, u8 *digest, int size); int ima_init_template(void); diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c index c35cfb5..414862e 100644 --- a/security/integrity/ima/ima_fs.c +++ b/security/integrity/ima/ima_fs.c @@ -99,7 +99,7 @@ static void ima_measurements_stop(struct seq_file *m, void *v) { } -static void ima_putc(struct seq_file *m, void *data, int datalen) +void ima_putc(struct seq_file *m, void *data, int datalen) { while (datalen--) seq_putc(m, *(char *)data++); @@ -167,7 +167,7 @@ static const struct file_operations ima_measurements_ops = { .release = seq_release, }; -static void ima_print_digest(struct seq_file *m, u8 *digest, int size) +void ima_print_digest(struct seq_file *m, u8 *digest, int size) { int i; diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c index 7e86783..8100422 100644 --- a/security/integrity/ima/ima_template.c +++ b/security/integrity/ima/ima_template.c @@ -13,15 +13,20 @@ * Helpers to manage template descriptors. */ #include "ima.h" +#include "ima_template_lib.h" static struct ima_template_desc defined_templates[] = { + {.name = IMA_TEMPLATE_IMA_NAME,.fmt = IMA_TEMPLATE_IMA_FMT}, }; static struct ima_template_field supported_fields[] = { + {.field_id = "d",.field_init = ima_eventdigest_init, +.field_show = ima_show_template_digest}, + {.field_id = "n",.field_init = ima_eventname_init, +.field_show = ima_show_template_string}, }; -static struct ima_template_field *ima_lookup_template_field( - const char *field_id) +static struct ima_template_field *lookup_template_field(const char *field_id) { int i; @@ -32,7 +37,7 @@ static struct ima_template_field *ima_lookup_template_field( return NULL; } -static int ima_template_fmt_size(char *template_fmt) +static int template_fmt_size(char *template_fmt) { char c; int template_fmt_len = strlen(template_fmt); @@ -53,7 +58,7 @@ static int template_desc_init_fields(char *template_fmt, int *num_fields) { char *c, *template_fmt_ptr = template_fmt; - int template_num_fields = ima_template_fmt_size(template_fmt); + int template_num_fields = template_fmt_size(template_fmt); int i,
Re: [PATCH] cpufreq: s3c64xx: Rename index to driver_data
On Monday, October 21, 2013 12:59:30 PM Viresh Kumar wrote: > On 15 October 2013 00:06, Charles Keepax > wrote: > > The index field of cpufreq_frequency_table has been renamed to driver > > data in this commit: > > > > commit 5070158804b5339c71809f5e673cea1cfacd804d > > cpufreq: rename index as driver_data in cpufreq_frequency_table > > > > This patch updates the s3c64xx driver to match. > > > > Signed-off-by: Charles Keepax > > --- > > drivers/cpufreq/s3c64xx-cpufreq.c |2 +- > > 1 files changed, 1 insertions(+), 1 deletions(-) > > > > diff --git a/drivers/cpufreq/s3c64xx-cpufreq.c > > b/drivers/cpufreq/s3c64xx-cpufreq.c > > index 8a72b0c..15631f9 100644 > > --- a/drivers/cpufreq/s3c64xx-cpufreq.c > > +++ b/drivers/cpufreq/s3c64xx-cpufreq.c > > @@ -166,7 +166,7 @@ static void __init > > s3c64xx_cpufreq_config_regulator(void) > > if (freq->frequency == CPUFREQ_ENTRY_INVALID) > > continue; > > > > - dvfs = _dvfs_table[freq->index]; > > + dvfs = _dvfs_table[freq->driver_data]; > > found = 0; > > > > for (i = 0; i < count; i++) { > > Acked-by: Viresh Kumar > > @Rafael: Can you please get this in 3.12 as a bug fix? It went in already, didn't it? -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 10/23] ima: ima_calc_boot_agregate must use SHA1
From: Dmitry Kasatkin With multiple hash algorithms, ima_hash_tfm is no longer guaranteed to be sha1. Need to force to use sha1. Changelog: - pass ima_digest_data to ima_calc_boot_aggregate() instead of char * (Roberto Sassu); - create an ima_digest_data structure in ima_add_boot_aggregate() (Roberto Sassu); - pass hash->algo to ima_alloc_tfm() (Roberto Sassu, reported by Dmitry). - "move hash definition in ima_add_boot_aggregate()" commit hunk to here. - sparse warning fix - Fengguang Wu Signed-off-by: Dmitry Kasatkin Signed-off-by: Roberto Sassu Signed-off-by: Mimi Zohar --- security/integrity/ima/ima.h| 2 +- security/integrity/ima/ima_crypto.c | 24 +--- security/integrity/ima/ima_init.c | 10 +- 3 files changed, 31 insertions(+), 5 deletions(-) diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 52393ed..e0e1cde 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -73,7 +73,7 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation, int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash); int ima_calc_buffer_hash(const void *data, int len, struct ima_digest_data *hash); -int ima_calc_boot_aggregate(char *digest); +int __init ima_calc_boot_aggregate(struct ima_digest_data *hash); void ima_add_violation(struct inode *inode, const unsigned char *filename, const char *op, const char *cause); int ima_init_crypto(void); diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c index e2be252..22be23f 100644 --- a/security/integrity/ima/ima_crypto.c +++ b/security/integrity/ima/ima_crypto.c @@ -184,16 +184,17 @@ static void __init ima_pcrread(int idx, u8 *pcr) /* * Calculate the boot aggregate hash */ -int __init ima_calc_boot_aggregate(char *digest) +static int __init ima_calc_boot_aggregate_tfm(char *digest, + struct crypto_shash *tfm) { u8 pcr_i[TPM_DIGEST_SIZE]; int rc, i; struct { struct shash_desc shash; - char ctx[crypto_shash_descsize(ima_shash_tfm)]; + char ctx[crypto_shash_descsize(tfm)]; } desc; - desc.shash.tfm = ima_shash_tfm; + desc.shash.tfm = tfm; desc.shash.flags = 0; rc = crypto_shash_init(); @@ -210,3 +211,20 @@ int __init ima_calc_boot_aggregate(char *digest) crypto_shash_final(, digest); return rc; } + +int __init ima_calc_boot_aggregate(struct ima_digest_data *hash) +{ + struct crypto_shash *tfm; + int rc; + + tfm = ima_alloc_tfm(hash->algo); + if (IS_ERR(tfm)) + return PTR_ERR(tfm); + + hash->length = crypto_shash_digestsize(tfm); + rc = ima_calc_boot_aggregate_tfm(hash->digest, tfm); + + ima_free_tfm(tfm); + + return rc; +} diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c index 9d0243c..77cd500 100644 --- a/security/integrity/ima/ima_init.c +++ b/security/integrity/ima/ima_init.c @@ -18,6 +18,7 @@ #include #include #include +#include #include "ima.h" /* name for boot aggregate entry */ @@ -46,6 +47,10 @@ static void __init ima_add_boot_aggregate(void) const char *audit_cause = "ENOMEM"; int result = -ENOMEM; int violation = 1; + struct { + struct ima_digest_data hdr; + char digest[TPM_DIGEST_SIZE]; + } hash; entry = kmalloc(sizeof(*entry), GFP_KERNEL); if (!entry) @@ -56,12 +61,15 @@ static void __init ima_add_boot_aggregate(void) IMA_EVENT_NAME_LEN_MAX); if (ima_used_chip) { violation = 0; - result = ima_calc_boot_aggregate(entry->template.digest); + hash.hdr.algo = HASH_ALGO_SHA1; + result = ima_calc_boot_aggregate(); if (result < 0) { audit_cause = "hashing_error"; kfree(entry); goto err_out; } + memcpy(entry->template.digest, hash.hdr.digest, + hash.hdr.length); } result = ima_store_template(entry, violation, NULL); if (result < 0) -- 1.8.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] atm: firestream: remove duplicate define
From: chas williams - CONTRACTOR Date: Mon, 21 Oct 2013 07:53:35 -0400 > Acked-by: Chas Williams > > On Mon, 21 Oct 2013 10:12:41 +0200 > Michael Opdenacker wrote: > >> This patch removes a duplicate define in drivers/atm/firestream.h >> >> Signed-off-by: Michael Opdenacker Applied. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] chelsio: remove duplicate defines
From: Michael Opdenacker Date: Mon, 21 Oct 2013 07:09:49 +0200 > This removes multiple duplicate definitions > in drivers/net/ethernet/chelsio/cxgb3/regs.h > > Signed-off-by: Michael Opdenacker Applied. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ethernet: moxa: remove duplicate includes
From: Michael Opdenacker Date: Sun, 20 Oct 2013 07:13:56 +0200 > Reported by "make includecheck" > > Tested that drivers/net/ethernet/moxa/moxart_ether.c still compiles > well on ARM > > Signed-off-by: Michael Opdenacker Applied. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] cgxb4: remove duplicate include in cxgb4.h
From: Michael Opdenacker Date: Sun, 20 Oct 2013 07:10:01 +0200 > Reported by "make includecheck" > > Tested that C sources including this file still compile well on x86 > > Signed-off-by: Michael Opdenacker Applied. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 20/23] ima: add Kconfig default measurement list template
This patch adds a Kconfig option to select the default IMA measurement list template. The 'ima' template limited the filedata hash to 20 bytes and the pathname to 255 charaters. The 'ima-ng' measurement list template permits larger hash digests and longer pathnames. Changelog: - keep 'select CRYPTO_HASH_INFO' in 'config IMA' section (Kconfig) (Roberto Sassu); - removed trailing whitespaces (Roberto Sassu). - Lindent fixes Signed-off-by: Mimi Zohar Signed-off-by: Roberto Sassu --- security/integrity/ima/Kconfig| 25 + security/integrity/ima/ima_template.c | 4 ++-- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig index e6628e7..de26cc8 100644 --- a/security/integrity/ima/Kconfig +++ b/security/integrity/ima/Kconfig @@ -46,6 +46,31 @@ config IMA_LSM_RULES help Disabling this option will disregard LSM based policy rules. +choice + prompt "Default template" + default IMA_NG_TEMPLATE + depends on IMA + help + Select the default IMA measurement template. + + The original 'ima' measurement list template contains a + hash, defined as 20 bytes, and a null terminated pathname, + limited to 255 characters. The 'ima-ng' measurement list + template permits both larger hash digests and longer + pathnames. + + config IMA_TEMPLATE + bool "ima" + config IMA_NG_TEMPLATE + bool "ima-ng (default)" +endchoice + +config IMA_DEFAULT_TEMPLATE + string + depends on IMA + default "ima" if IMA_TEMPLATE + default "ima-ng" if IMA_NG_TEMPLATE + config IMA_APPRAISE bool "Appraise integrity measurements" depends on IMA diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c index 1c4cf19..c28ff9b 100644 --- a/security/integrity/ima/ima_template.c +++ b/security/integrity/ima/ima_template.c @@ -127,8 +127,8 @@ static int init_defined_templates(void) struct ima_template_desc *ima_template_desc_current(void) { if (!ima_template) - ima_template = lookup_template_desc(IMA_TEMPLATE_IMA_NAME); - + ima_template = + lookup_template_desc(CONFIG_IMA_DEFAULT_TEMPLATE); return ima_template; } -- 1.8.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 09/23] ima: support arbitrary hash algorithms in ima_calc_buffer_hash
From: Dmitry Kasatkin ima_calc_buffer_hash will be used with different hash algorithms. This patch provides support for arbitrary hash algorithms in ima_calc_buffer_hash. Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_api.c| 3 +++ security/integrity/ima/ima_crypto.c | 28 ++-- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index 2cc5dcc..bc1d128 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -18,6 +18,7 @@ #include #include #include +#include #include "ima.h" static const char *IMA_TEMPLATE_NAME = "ima"; @@ -54,6 +55,8 @@ int ima_store_template(struct ima_template_entry *entry, entry->template_len = sizeof(entry->template); if (!violation) { + /* this function uses default algo */ + hash.hdr.algo = HASH_ALGO_SHA1; result = ima_calc_buffer_hash(>template, entry->template_len, ); if (result < 0) { diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c index e5d3ebf..e2be252 100644 --- a/security/integrity/ima/ima_crypto.c +++ b/security/integrity/ima/ima_crypto.c @@ -139,23 +139,39 @@ int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash) /* * Calculate the hash of a given buffer */ -int ima_calc_buffer_hash(const void *buf, int len, struct ima_digest_data *hash) +static int ima_calc_buffer_hash_tfm(const void *buf, int len, + struct ima_digest_data *hash, + struct crypto_shash *tfm) { struct { struct shash_desc shash; - char ctx[crypto_shash_descsize(ima_shash_tfm)]; + char ctx[crypto_shash_descsize(tfm)]; } desc; - desc.shash.tfm = ima_shash_tfm; + desc.shash.tfm = tfm; desc.shash.flags = 0; - /* this function uses default algo */ - hash->algo = ima_hash_algo; - hash->length = crypto_shash_digestsize(ima_shash_tfm); + hash->length = crypto_shash_digestsize(tfm); return crypto_shash_digest(, buf, len, hash->digest); } +int ima_calc_buffer_hash(const void *buf, int len, struct ima_digest_data *hash) +{ + struct crypto_shash *tfm; + int rc; + + tfm = ima_alloc_tfm(hash->algo); + if (IS_ERR(tfm)) + return PTR_ERR(tfm); + + rc = ima_calc_buffer_hash_tfm(buf, len, hash, tfm); + + ima_free_tfm(tfm); + + return rc; +} + static void __init ima_pcrread(int idx, u8 *pcr) { if (!ima_used_chip) -- 1.8.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 14/23] ima: new templates management mechanism
From: Roberto Sassu The original 'ima' template is fixed length, containing the filedata hash and pathname. The filedata hash is limited to 20 bytes (md5/sha1). The pathname is a null terminated string, limited to 255 characters. To overcome these limitations and to add additional file metadata, it is necessary to extend the current version of IMA by defining additional templates. The main reason to introduce this feature is that, each time a new template is defined, the functions that generate and display the measurement list would include the code for handling a new format and, thus, would significantly grow over time. This patch set solves this problem by separating the template management from the remaining IMA code. The core of this solution is the definition of two new data structures: a template descriptor, to determine which information should be included in the measurement list, and a template field, to generate and display data of a given type. To define a new template field, developers define the field identifier and implement two functions, init() and show(), respectively to generate and display measurement entries. Initially, this patch set defines the following template fields (support for additional data types will be added later): - 'd': the digest of the event (i.e. the digest of a measured file), calculated with the SHA1 or MD5 hash algorithm; - 'n': the name of the event (i.e. the file name), with size up to 255 bytes; - 'd-ng': the digest of the event, calculated with an arbitrary hash algorithm (field format: [:]digest, where the digest prefix is shown only if the hash algorithm is not SHA1 or MD5); - 'n-ng': the name of the event, without size limitations. Defining a new template descriptor requires specifying the template format, a string of field identifiers separated by the '|' character. This patch set defines the following template descriptors: - "ima": its format is 'd|n'; - "ima-ng" (default): its format is 'd-ng|n-ng' Further details about the new template architecture can be found in Documentation/security/IMA-templates.txt. Changelog: - don't defer calling ima_init_template() - Mimi - don't define ima_lookup_template_desc() until used - Mimi - squashed with documentation patch - Mimi Signed-off-by: Roberto Sassu Signed-off-by: Mimi Zohar --- Documentation/security/00-INDEX | 2 + Documentation/security/IMA-templates.txt | 87 security/integrity/ima/Makefile | 2 +- security/integrity/ima/ima.h | 29 security/integrity/ima/ima_init.c| 4 ++ security/integrity/ima/ima_template.c| 112 +++ 6 files changed, 235 insertions(+), 1 deletion(-) create mode 100644 Documentation/security/IMA-templates.txt create mode 100644 security/integrity/ima/ima_template.c diff --git a/Documentation/security/00-INDEX b/Documentation/security/00-INDEX index 414235c..45c82fd 100644 --- a/Documentation/security/00-INDEX +++ b/Documentation/security/00-INDEX @@ -22,3 +22,5 @@ keys.txt - description of the kernel key retention service. tomoyo.txt - documentation on the TOMOYO Linux Security Module. +IMA-templates.txt + - documentation on the template management mechanism for IMA. diff --git a/Documentation/security/IMA-templates.txt b/Documentation/security/IMA-templates.txt new file mode 100644 index 000..a777e5f --- /dev/null +++ b/Documentation/security/IMA-templates.txt @@ -0,0 +1,87 @@ + IMA Template Management Mechanism + + + INTRODUCTION + +The original 'ima' template is fixed length, containing the filedata hash +and pathname. The filedata hash is limited to 20 bytes (md5/sha1). +The pathname is a null terminated string, limited to 255 characters. +To overcome these limitations and to add additional file metadata, it is +necessary to extend the current version of IMA by defining additional +templates. For example, information that could be possibly reported are +the inode UID/GID or the LSM labels either of the inode and of the process +that is accessing it. + +However, the main problem to introduce this feature is that, each time +a new template is defined, the functions that generate and display +the measurements list would include the code for handling a new format +and, thus, would significantly grow over the time. + +The proposed solution solves this problem by separating the template +management from the remaining IMA code. The core of this solution is the +definition of two new data structures: a template descriptor, to determine +which information should be included in the measurement list; a template +field, to generate and display data of a given type. + +Managing templates with these structures is very simple. To support +a new data type, developers define the field identifier and implement +two functions, init() and show(), respectively to generate
[PATCH v2 19/23] ima: defer determining the appraisal hash algorithm for 'ima' template
From: Roberto Sassu The same hash algorithm should be used for calculating the file data hash for the IMA measurement list, as for appraising the file data integrity. (The appraise hash algorithm is stored in the 'security.ima' extended attribute.) The exception is when the reference file data hash digest, stored in the extended attribute, is larger than the one supported by the template. In this case, the file data hash needs to be calculated twice, once for the measurement list and, again, for appraisal. Signed-off-by: Roberto Sassu Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_main.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 5e8b1f7..0b11bb4 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -145,6 +145,7 @@ static int process_measurement(struct file *file, const char *filename, { struct inode *inode = file_inode(file); struct integrity_iint_cache *iint; + struct ima_template_desc *template_desc = ima_template_desc_current(); char *pathbuf = NULL; const char *pathname = NULL; int rc = -ENOMEM, action, must_appraise, _func; @@ -188,7 +189,10 @@ static int process_measurement(struct file *file, const char *filename, goto out_digsig; } - if (action & IMA_APPRAISE_SUBMASK) + if (strcmp(template_desc->name, IMA_TEMPLATE_IMA_NAME) == 0) { + if (action & IMA_APPRAISE_SUBMASK) + xattr_ptr = _value; + } else xattr_ptr = _value; rc = ima_collect_measurement(iint, file, xattr_ptr, _len); -- 1.8.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 17/23] ima: switch to new template management mechanism
From: Roberto Sassu This patch performs the switch to the new template mechanism by modifying the functions ima_alloc_init_template(), ima_measurements_show() and ima_ascii_measurements_show(). The old function ima_template_show() was removed as it is no longer needed. Also, if the template descriptor used to generate a measurement entry is not 'ima', the whole length of field data stored for an entry is provided before the data itself through the binary_runtime_measurement interface. Changelog: - unnecessary to use strncmp() (Mimi Zohar) - create new variable 'field' in ima_alloc_init_template() (Roberto Sassu) - use GFP_NOFS flag in ima_alloc_init_template() (Roberto Sassu) - new variable 'num_fields' in ima_store_template() (Roberto Sassu, proposed by Mimi Zohar) - rename ima_calc_buffer_hash/template_hash() to ima_calc_field_array_hash(), something more generic (Mimi, requested by Dmitry) - sparse error fix - Fengguang Wu - fix lindent warnings - always include the field length in the template data length - include the template field length variable size in the template data length - include both the template field data and field length in the template digest calculation. Simplifies verifying the template digest. (Mimi) Signed-off-by: Roberto Sassu Signed-off-by: Mimi Zohar --- security/integrity/ima/ima.h | 19 - security/integrity/ima/ima_api.c | 75 --- security/integrity/ima/ima_crypto.c | 34 security/integrity/ima/ima_fs.c | 54 - security/integrity/ima/ima_template.c | 22 ++ 5 files changed, 107 insertions(+), 97 deletions(-) diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index e1f081d..72d013e 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -72,17 +72,11 @@ struct ima_template_desc { struct ima_template_field **fields; }; -/* IMA inode template definition */ -struct ima_template_data { - u8 digest[IMA_DIGEST_SIZE]; /* sha1/md5 measurement hash */ - char file_name[IMA_EVENT_NAME_LEN_MAX + 1]; /* name + \0 */ -}; - struct ima_template_entry { u8 digest[TPM_DIGEST_SIZE]; /* sha1 or md5 measurement hash */ - const char *template_name; - int template_len; - struct ima_template_data template; + struct ima_template_desc *template_desc; /* template descriptor */ + u32 template_data_len; + struct ima_field_data template_data[0]; /* template related data */ }; struct ima_queue_entry { @@ -102,14 +96,16 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation, const char *op, struct inode *inode, const unsigned char *filename); int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash); -int ima_calc_buffer_hash(const void *data, int len, -struct ima_digest_data *hash); +int ima_calc_field_array_hash(struct ima_field_data *field_data, int num_fields, + struct ima_digest_data *hash); int __init ima_calc_boot_aggregate(struct ima_digest_data *hash); void ima_add_violation(struct file *file, const unsigned char *filename, const char *op, const char *cause); int ima_init_crypto(void); void ima_putc(struct seq_file *m, void *data, int datalen); void ima_print_digest(struct seq_file *m, u8 *digest, int size); +struct ima_template_desc *ima_template_desc_current(void); +int ima_init_template(void); int ima_init_template(void); @@ -146,7 +142,6 @@ int ima_alloc_init_template(struct integrity_iint_cache *iint, struct ima_template_entry **entry); int ima_store_template(struct ima_template_entry *entry, int violation, struct inode *inode, const unsigned char *filename); -void ima_template_show(struct seq_file *m, void *e, enum ima_show_type show); const char *ima_d_path(struct path *path, char **pathbuf); /* rbtree tree calls to lookup, insert, delete diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index 29dd43d..baa3481 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -21,8 +21,6 @@ #include #include "ima.h" -static const char *IMA_TEMPLATE_NAME = "ima"; - /* * ima_alloc_init_template - create and initialize a new template entry */ @@ -30,52 +28,32 @@ int ima_alloc_init_template(struct integrity_iint_cache *iint, struct file *file, const unsigned char *filename, struct ima_template_entry **entry) { - struct ima_template_entry *e; - int result = 0; + struct ima_template_desc *template_desc = ima_template_desc_current(); + int i, result = 0; - e = kzalloc(sizeof(**entry), GFP_NOFS); - if (!e) + *entry = kzalloc(sizeof(**entry) +
Re: [PATCH] Revert "bridge: only expire the mdb entry when query is received"
From: Linus Lüssing Date: Sun, 20 Oct 2013 00:58:57 +0200 > While this commit was a good attempt to fix issues occuring when no > multicast querier is present, this commit still has two more issues: > > 1) There are cases where mdb entries do not expire even if there is a > querier present. The bridge will unnecessarily continue flooding > multicast packets on the according ports. > > 2) Never removing an mdb entry could be exploited for a Denial of > Service by an attacker on the local link, slowly, but steadily eating up > all memory. > > Actually, this commit became obsolete with > "bridge: disable snooping if there is no querier" (b00589af3b) > which included fixes for a few more cases. > > Therefore reverting the following commits (the commit stated in the > commit message plus three of its follow up fixes): > > --- > Revert "bridge: update mdb expiration timer upon reports." > This reverts commit f144febd93d5ee534fdf23505ab091b2b9088edc. > Revert "bridge: do not call setup_timer() multiple times" > This reverts commit 1faabf2aab1fdaa1ace4e8c829d1b9cf7bfec2f1. > Revert "bridge: fix some kernel warning in multicast timer" > This reverts commit c7e8e8a8f7a70b343ca1e0f90a31e35ab2d16de1. > Revert "bridge: only expire the mdb entry when query is received" > This reverts commit 9f00b2e7cf241fa389733d41b615efdaa2cb0f5b. > --- Cong, and other bridge folks, please review this revert. Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 21/23] ima: define kernel parameter 'ima_template=' to change configured default
From: Roberto Sassu This patch allows users to specify from the kernel command line the template descriptor, among those defined, that will be used to generate and display measurement entries. If an user specifies a wrong template, IMA reverts to the template descriptor set in the kernel configuration. Signed-off-by: Roberto Sassu Signed-off-by: Mimi Zohar --- Documentation/kernel-parameters.txt | 5 + security/integrity/ima/ima_template.c | 31 +++ 2 files changed, 36 insertions(+) diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt index 1a036cd9..2b78cb5 100644 --- a/Documentation/kernel-parameters.txt +++ b/Documentation/kernel-parameters.txt @@ -1190,6 +1190,11 @@ bytes respectively. Such letter suffixes can also be entirely omitted. programs exec'd, files mmap'd for exec, and all files opened for read by uid=0. + ima_template= [IMA] + Select one of defined IMA measurements template formats. + Formats: { "ima" | "ima-ng" } + Default: "ima-ng" + init= [KNL] Format: Run specified binary instead of /sbin/init as init diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c index c28ff9b..0002214 100644 --- a/security/integrity/ima/ima_template.c +++ b/security/integrity/ima/ima_template.c @@ -12,6 +12,8 @@ * File: ima_template.c * Helpers to manage template descriptors. */ +#include + #include "ima.h" #include "ima_template_lib.h" @@ -32,6 +34,35 @@ static struct ima_template_field supported_fields[] = { }; static struct ima_template_desc *ima_template; +static struct ima_template_desc *lookup_template_desc(const char *name); + +static int __init ima_template_setup(char *str) +{ + struct ima_template_desc *template_desc; + int template_len = strlen(str); + + /* +* Verify that a template with the supplied name exists. +* If not, use CONFIG_IMA_DEFAULT_TEMPLATE. +*/ + template_desc = lookup_template_desc(str); + if (!template_desc) + return 1; + + /* +* Verify whether the current hash algorithm is supported +* by the 'ima' template. +*/ + if (template_len == 3 && strcmp(str, IMA_TEMPLATE_IMA_NAME) == 0 && + ima_hash_algo != HASH_ALGO_SHA1 && ima_hash_algo != HASH_ALGO_MD5) { + pr_err("IMA: template does not support hash alg\n"); + return 1; + } + + ima_template = template_desc; + return 1; +} +__setup("ima_template=", ima_template_setup); static struct ima_template_desc *lookup_template_desc(const char *name) { -- 1.8.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] xfs: fix possible NULL dereference
On Mon, Oct 21, 2013 at 03:58:23PM -0500, Eric Sandeen wrote: > On 10/21/13 1:32 PM, Geyslan G. Bem wrote: > > This patch puts a 'break' in the true branch, avoiding the 'icptr->ic_next' > > dereferencing. > > Reviewed-by: Eric Sandeen Actually, NACK. > Hm, yeah - cmn_err(CE_PANIC, " " ); used to BUG_ON, but the newer > xfs_emerg() doesn't. > > Dave, was that intentional? Of course it was. ;) xfs_emerg() is only called from the debug code in xlog_verify_iclog(), xlog_verify_tail_lsn and assfail(). In the case of assfail(), it has it's own BUG() call, so it does everything just fine. In the case of xlog_verify_iclog() when icptr is NULL, it will panic immediately after the message is printed, just like the old code. i.e. this patch isn't fixing anything we need fixed. > I wonder if there are more spots after xfs_emerg()'s which aren't > defensive, because the code used to just panic there. As for the rest of the calls in xlog_verify_iclog, they are checking things that aren't immediately fatal, but indication that iclog corruption has already occurred. It's debug code, so we could add "panic immediately" code, but personally I'd prefer to see the error message being printed and then have it continue like a production system would so that we can see the types of crashes normal kernels will see as a result of iclog memory corruption As for xlog_verify_tail_lsn(), that's an important informational message indicating we might be leaking log space. It's not immediately fatal, but if we see it and then have a log space hang... So, really, none of the callers really need xfs_emerg to panic like CE_PANIC used to. The one case where it might be useful (i.e this patch) we panic immediately anyway Cheers, Dave. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 23/23] ima: provide hash algo info in the xattr
From: Dmitry Kasatkin All files labeled with 'security.ima' hashes, are hashed using the same hash algorithm. Changing from one hash algorithm to another, requires relabeling the filesystem. This patch defines a new xattr type, which includes the hash algorithm, permitting different files to be hashed with different algorithms. Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_appraise.c | 61 +++ security/integrity/integrity.h| 13 +++- 2 files changed, 59 insertions(+), 15 deletions(-) diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index 116630c..734e946 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -15,6 +15,7 @@ #include #include #include +#include #include "ima.h" @@ -45,10 +46,22 @@ int ima_must_appraise(struct inode *inode, int mask, enum ima_hooks func) static int ima_fix_xattr(struct dentry *dentry, struct integrity_iint_cache *iint) { - iint->ima_hash->type = IMA_XATTR_DIGEST; - return __vfs_setxattr_noperm(dentry, XATTR_NAME_IMA, ->ima_hash->type, -1 + iint->ima_hash->length, 0); + int rc, offset; + u8 algo = iint->ima_hash->algo; + + if (algo <= HASH_ALGO_SHA1) { + offset = 1; + iint->ima_hash->xattr.sha1.type = IMA_XATTR_DIGEST; + } else { + offset = 0; + iint->ima_hash->xattr.ng.type = IMA_XATTR_DIGEST_NG; + iint->ima_hash->xattr.ng.algo = algo; + } + rc = __vfs_setxattr_noperm(dentry, XATTR_NAME_IMA, + >ima_hash->xattr.data[offset], + (sizeof(iint->ima_hash->xattr) - offset) + + iint->ima_hash->length, 0); + return rc; } /* Return specific func appraised cached result */ @@ -112,15 +125,31 @@ void ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value, int xattr_len, { struct signature_v2_hdr *sig; - if (!xattr_value || xattr_len < 0 || xattr_len <= 1 + sizeof(*sig)) + if (!xattr_value || xattr_len < 2) return; - sig = (typeof(sig)) xattr_value->digest; - - if (xattr_value->type != EVM_IMA_XATTR_DIGSIG || sig->version != 2) - return; - - hash->algo = sig->hash_algo; + switch (xattr_value->type) { + case EVM_IMA_XATTR_DIGSIG: + sig = (typeof(sig))xattr_value; + if (sig->version != 2 || xattr_len <= sizeof(*sig)) + return; + hash->algo = sig->hash_algo; + break; + case IMA_XATTR_DIGEST_NG: + hash->algo = xattr_value->digest[0]; + break; + case IMA_XATTR_DIGEST: + /* this is for backward compatibility */ + if (xattr_len == 21) { + unsigned int zero = 0; + if (!memcmp(_value->digest[16], , 4)) + hash->algo = HASH_ALGO_MD5; + else + hash->algo = HASH_ALGO_SHA1; + } else if (xattr_len == 17) + hash->algo = HASH_ALGO_MD5; + break; + } } int ima_read_xattr(struct dentry *dentry, @@ -153,7 +182,7 @@ int ima_appraise_measurement(int func, struct integrity_iint_cache *iint, enum integrity_status status = INTEGRITY_UNKNOWN; const char *op = "appraise_data"; char *cause = "unknown"; - int rc = xattr_len; + int rc = xattr_len, hash_start = 0; if (!ima_appraise) return 0; @@ -180,17 +209,21 @@ int ima_appraise_measurement(int func, struct integrity_iint_cache *iint, goto out; } switch (xattr_value->type) { + case IMA_XATTR_DIGEST_NG: + /* first byte contains algorithm id */ + hash_start = 1; case IMA_XATTR_DIGEST: if (iint->flags & IMA_DIGSIG_REQUIRED) { cause = "IMA signature required"; status = INTEGRITY_FAIL; break; } - if (xattr_len - 1 >= iint->ima_hash->length) + if (xattr_len - sizeof(xattr_value->type) - hash_start >= + iint->ima_hash->length) /* xattr length may be longer. md5 hash in previous version occupied 20 bytes in xattr, instead of 16 */ - rc = memcmp(xattr_value->digest, + rc = memcmp(_value->digest[hash_start], iint->ima_hash->digest, iint->ima_hash->length);
Re: [PATCH] cpufreq: intel_pstate: fix possible integer overflow
On 10/21/2013 03:47 PM, Rafael J. Wysocki wrote: On Monday, October 21, 2013 08:56:22 AM Dirk Brandewie wrote: On 10/19/2013 08:31 PM, Geyslan G. Bem wrote: The expression 'pstate << 8' is evaluated using 32-bit arithmetic while 'val' expects an expression of type u64. Signed-off-by: Geyslan G. Bem Acked-by: Dirk Brandewie Actually, isn't (pstate << 8) guaranteed not to overflow? Yes, I was assuming this was caught by a static checking tool. I didn't see a downside to giving the compilier complete information. --- drivers/cpufreq/intel_pstate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c index badf620..43446b5 100644 --- a/drivers/cpufreq/intel_pstate.c +++ b/drivers/cpufreq/intel_pstate.c @@ -395,7 +395,7 @@ static void intel_pstate_set_pstate(struct cpudata *cpu, int pstate) trace_cpu_frequency(pstate * 10, cpu->cpu); cpu->pstate.current_pstate = pstate; - val = pstate << 8; + val = (u64)pstate << 8; if (limits.no_turbo) val |= (u64)1 << 32; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 22/23] ima: enable support for larger default filedata hash algorithms
The IMA measurement list contains two hashes - a template data hash and a filedata hash. The template data hash is committed to the TPM, which is limited, by the TPM v1.2 specification, to 20 bytes. The filedata hash is defined as 20 bytes as well. Now that support for variable length measurement list templates was added, the filedata hash is not limited to 20 bytes. This patch adds Kconfig support for defining larger default filedata hash algorithms and replacing the builtin default with one specified on the kernel command line. contains a list of hash algorithms. The Kconfig default hash algorithm is a subset of this list, but any hash algorithm included in the list can be specified at boot, using the 'ima_hash=' kernel command line option. Changelog v2: - update Kconfig Changelog: - support hashes that are configured - use generic HASH_ALGO_ definitions - add Kconfig support - hash_setup must be called only once (Dmitry) - removed trailing whitespaces (Roberto Sassu) Signed-off-by: Mimi Zohar Signed-off-by: Roberto Sassu --- Documentation/kernel-parameters.txt | 6 +- security/integrity/ima/Kconfig | 35 +++ security/integrity/ima/ima_main.c | 26 -- 3 files changed, 64 insertions(+), 3 deletions(-) diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt index 2b78cb5..1e8761c 100644 --- a/Documentation/kernel-parameters.txt +++ b/Documentation/kernel-parameters.txt @@ -1181,9 +1181,13 @@ bytes respectively. Such letter suffixes can also be entirely omitted. owned by uid=0. ima_hash= [IMA] - Format: { "sha1" | "md5" } + Format: { md5 | sha1 | rmd160 | sha256 | sha384 + | sha512 | ... } default: "sha1" + The list of supported hash algorithms is defined + in crypto/hash_info.h. + ima_tcb [IMA] Load a policy which meets the needs of the Trusted Computing Base. This means IMA will measure all diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig index de26cc8..351a58e 100644 --- a/security/integrity/ima/Kconfig +++ b/security/integrity/ima/Kconfig @@ -71,6 +71,41 @@ config IMA_DEFAULT_TEMPLATE default "ima" if IMA_TEMPLATE default "ima-ng" if IMA_NG_TEMPLATE +choice + prompt "Default integrity hash algorithm" + default IMA_DEFAULT_HASH_SHA1 + depends on IMA + help + Select the default hash algorithm used for the measurement + list, integrity appraisal and audit log. The compiled default + hash algorithm can be overwritten using the kernel command + line 'ima_hash=' option. + + config IMA_DEFAULT_HASH_SHA1 + bool "SHA1 (default)" + depends on CRYPTO_SHA1 + + config IMA_DEFAULT_HASH_SHA256 + bool "SHA256" + depends on CRYPTO_SHA256 && !IMA_TEMPLATE + + config IMA_DEFAULT_HASH_SHA512 + bool "SHA512" + depends on CRYPTO_SHA512 && !IMA_TEMPLATE + + config IMA_DEFAULT_HASH_WP512 + bool "WP512" + depends on CRYPTO_WP512 && !IMA_TEMPLATE +endchoice + +config IMA_DEFAULT_HASH + string + depends on IMA + default "sha1" if IMA_DEFAULT_HASH_SHA1 + default "sha256" if IMA_DEFAULT_HASH_SHA256 + default "sha512" if IMA_DEFAULT_HASH_SHA512 + default "wp512" if IMA_DEFAULT_HASH_WP512 + config IMA_APPRAISE bool "Appraise integrity measurements" depends on IMA diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 0b11bb4..14d4cb5 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -37,11 +37,32 @@ int ima_appraise; #endif int ima_hash_algo = HASH_ALGO_SHA1; +static int hash_setup_done; static int __init hash_setup(char *str) { - if (strncmp(str, "md5", 3) == 0) - ima_hash_algo = HASH_ALGO_MD5; + struct ima_template_desc *template_desc = ima_template_desc_current(); + int i; + + if (hash_setup_done) + return 1; + + if (strcmp(template_desc->name, IMA_TEMPLATE_IMA_NAME) == 0) { + if (strncmp(str, "sha1", 4) == 0) + ima_hash_algo = HASH_ALGO_SHA1; + else if (strncmp(str, "md5", 3) == 0) + ima_hash_algo = HASH_ALGO_MD5; + goto out; + } + + for (i = 0; i < HASH_ALGO__LAST; i++) { + if (strcmp(str, hash_algo_name[i]) == 0) { + ima_hash_algo = i; + break; + } + } +out: + hash_setup_done = 1; return 1; } __setup("ima_hash=", hash_setup); @@ -306,6 +327,7 @@
[PATCH v2 16/23] ima: define new template ima-ng and template fields d-ng and n-ng
From: Roberto Sassu This patch adds support for the new template 'ima-ng', whose format is defined as 'd-ng|n-ng'. These new field definitions remove the size limitations of the original 'ima' template. Further, the 'd-ng' field prefixes the inode digest with the hash algorithim, when displaying the new larger digest sizes. Change log: - scripts/Lindent fixes - Mimi - "always true comparison" - reported by Fengguang Wu, resolved Dmitry - initialize hash_algo variable to HASH_ALGO__LAST - always prefix digest with hash algorithm - Mimi Signed-off-by: Roberto Sassu Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_template.c | 7 +- security/integrity/ima/ima_template_lib.c | 152 ++ security/integrity/ima/ima_template_lib.h | 8 ++ 3 files changed, 150 insertions(+), 17 deletions(-) diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c index 8100422..bf38d1a 100644 --- a/security/integrity/ima/ima_template.c +++ b/security/integrity/ima/ima_template.c @@ -16,7 +16,8 @@ #include "ima_template_lib.h" static struct ima_template_desc defined_templates[] = { - {.name = IMA_TEMPLATE_IMA_NAME,.fmt = IMA_TEMPLATE_IMA_FMT}, + {.name = IMA_TEMPLATE_IMA_NAME, .fmt = IMA_TEMPLATE_IMA_FMT}, + {.name = "ima-ng",.fmt = "d-ng|n-ng"}, }; static struct ima_template_field supported_fields[] = { @@ -24,6 +25,10 @@ static struct ima_template_field supported_fields[] = { .field_show = ima_show_template_digest}, {.field_id = "n",.field_init = ima_eventname_init, .field_show = ima_show_template_string}, + {.field_id = "d-ng",.field_init = ima_eventdigest_ng_init, +.field_show = ima_show_template_digest_ng}, + {.field_id = "n-ng",.field_init = ima_eventname_ng_init, +.field_show = ima_show_template_string}, }; static struct ima_template_field *lookup_template_field(const char *field_id) diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c index e13fc7c..7d84144 100644 --- a/security/integrity/ima/ima_template_lib.c +++ b/security/integrity/ima/ima_template_lib.c @@ -12,9 +12,25 @@ * File: ima_template_lib.c * Library of supported template fields. */ +#include + #include "ima_template_lib.h" -enum data_formats { DATA_FMT_DIGEST = 0, DATA_FMT_EVENT_NAME, DATA_FMT_STRING }; +static bool ima_template_hash_algo_allowed(u8 algo) +{ + if (algo == HASH_ALGO_SHA1 || algo == HASH_ALGO_MD5) + return true; + + return false; +} + +enum data_formats { + DATA_FMT_DIGEST = 0, + DATA_FMT_DIGEST_WITH_ALGO, + DATA_FMT_EVENT_NAME, + DATA_FMT_STRING +}; + static int ima_write_template_field_data(const void *data, const u32 datalen, enum data_formats datafmt, struct ima_field_data *field_data) @@ -62,12 +78,22 @@ static void ima_show_template_data_ascii(struct seq_file *m, enum data_formats datafmt, struct ima_field_data *field_data) { + u8 *buf_ptr = field_data->data, buflen = field_data->len; + switch (datafmt) { + case DATA_FMT_DIGEST_WITH_ALGO: + buf_ptr = strnchr(field_data->data, buflen, ':'); + if (buf_ptr != field_data->data) + seq_printf(m, "%s", field_data->data); + + /* skip ':' and '\0' */ + buf_ptr += 2; + buflen -= buf_ptr - field_data->data; case DATA_FMT_DIGEST: - ima_print_digest(m, field_data->data, field_data->len); + ima_print_digest(m, buf_ptr, buflen); break; case DATA_FMT_STRING: - seq_printf(m, "%s", field_data->data); + seq_printf(m, "%s", buf_ptr); break; default: break; @@ -108,14 +134,59 @@ void ima_show_template_digest(struct seq_file *m, enum ima_show_type show, ima_show_template_field_data(m, show, DATA_FMT_DIGEST, field_data); } +void ima_show_template_digest_ng(struct seq_file *m, enum ima_show_type show, +struct ima_field_data *field_data) +{ + ima_show_template_field_data(m, show, DATA_FMT_DIGEST_WITH_ALGO, +field_data); +} + void ima_show_template_string(struct seq_file *m, enum ima_show_type show, struct ima_field_data *field_data) { ima_show_template_field_data(m, show, DATA_FMT_STRING, field_data); } +static int ima_eventdigest_init_common(u8 *digest, u32 digestsize, u8 hash_algo, + struct ima_field_data *field_data, + bool size_limit) +{ + /* +* digest formats: +* - DATA_FMT_DIGEST: digest +* -
[PATCH v2 18/23] ima: add audit log support for larger hashes
Different files might be signed based on different hash algorithms. This patch prefixes the audit log measurement hash with the hash algorithm. Changelog: - use generic HASH_ALGO defintions - use ':' as delimiter between the hash algorithm and the digest (Roberto Sassu) Signed-off-by: Mimi Zohar Signed-off-by: Roberto Sassu --- security/integrity/ima/ima_api.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index baa3481..f22725e 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -287,6 +287,12 @@ void ima_audit_measurement(struct integrity_iint_cache *iint, audit_log_format(ab, "file="); audit_log_untrustedstring(ab, filename); audit_log_format(ab, " hash="); + if (iint->ima_hash->algo != HASH_ALGO_SHA1 && + iint->ima_hash->algo != HASH_ALGO_MD5) { + audit_log_untrustedstring(ab, + hash_algo_name[iint->ima_hash->algo]); + audit_log_format(ab, ":"); + } audit_log_untrustedstring(ab, hash); audit_log_task_info(ab, current); -- 1.8.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 08/23] ima: provide dedicated hash algo allocation function
From: Dmitry Kasatkin This patch provides dedicated hash algo allocation and deallocation function which can be used by different clients. Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_crypto.c | 43 + 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c index 872c669..e5d3ebf 100644 --- a/security/integrity/ima/ima_crypto.c +++ b/security/integrity/ima/ima_crypto.c @@ -39,6 +39,28 @@ int ima_init_crypto(void) return 0; } +static struct crypto_shash *ima_alloc_tfm(enum hash_algo algo) +{ + struct crypto_shash *tfm = ima_shash_tfm; + int rc; + + if (algo != ima_hash_algo && algo < HASH_ALGO__LAST) { + tfm = crypto_alloc_shash(hash_algo_name[algo], 0, 0); + if (IS_ERR(tfm)) { + rc = PTR_ERR(tfm); + pr_err("Can not allocate %s (reason: %d)\n", + hash_algo_name[algo], rc); + } + } + return tfm; +} + +static void ima_free_tfm(struct crypto_shash *tfm) +{ + if (tfm != ima_shash_tfm) + crypto_free_shash(tfm); +} + /* * Calculate the MD5/SHA1 file digest */ @@ -57,6 +79,8 @@ static int ima_calc_file_hash_tfm(struct file *file, desc.shash.tfm = tfm; desc.shash.flags = 0; + hash->length = crypto_shash_digestsize(tfm); + rc = crypto_shash_init(); if (rc != 0) return rc; @@ -98,25 +122,16 @@ out: int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash) { - struct crypto_shash *tfm = ima_shash_tfm; + struct crypto_shash *tfm; int rc; - if (hash->algo != ima_hash_algo && hash->algo < HASH_ALGO__LAST) { - tfm = crypto_alloc_shash(hash_algo_name[hash->algo], 0, 0); - if (IS_ERR(tfm)) { - rc = PTR_ERR(tfm); - pr_err("Can not allocate %s (reason: %d)\n", - hash_algo_name[hash->algo], rc); - return rc; - } - } - - hash->length = crypto_shash_digestsize(tfm); + tfm = ima_alloc_tfm(hash->algo); + if (IS_ERR(tfm)) + return PTR_ERR(tfm); rc = ima_calc_file_hash_tfm(file, hash, tfm); - if (tfm != ima_shash_tfm) - crypto_free_shash(tfm); + ima_free_tfm(tfm); return rc; } -- 1.8.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 00/23] ima: larger digests and extensible template support
This patch set adds support for additional hash algorithms with larger digests, as well as support for additional file metadata in the IMA measurement list. The existing IMA measurement list entries, which are exposed to userspace via the securityfs ascii/binary_runtime_measurement lists, are fixed length, containing a file data hash, limited to a 20 byte digest, and a pathname, limited to 255 characters. Adding larger digest support for signature verification, without the template changes, would result in hashing the file twice, once for appraising the file signature and, again, for the measurement list. This patch set defines an extensible template architecture with support for larger hash algorithms. A description of the new template architecture is described in the "ima: new templates management mechanism" patch description and, with more detail, in Documentation/security/IMA-templates.txt. The two initial templates defined are: the original 'ima', for backwards compatibility, and 'ima-ng', which eliminates the digest and pathname size limitations. Additional templates, that include other file metadata (eg. uid/gid, LSM subject/object labels, file data signatures) will be posted separately. Two changes were made, since posting this patch set back in July http://marc.info/?l=linux-security-module=137410629309961=2. Namely, the measurement list can now be walked and verified, without understanding the template field data specifics; and "mutable" files can be labeled based on different hash algorithms. Walking and verifying the measurement list without understanding the template field data specifics, will allow new templates to be defined in the kernel, without breaking userspace applications. Defining a new extended attribute format, which includes the file hash algorithm, eliminates the need for relabeling "mutable" files. Changelog: - fix lindent, sparse, checkpath warnings/errors - define a new extended attribute type, which includes the file data hash algorithm. - template changes: - simplify walking the binary measurement list - simplify calculating the template data hash - simplify parsing measurement entries by always prefixing the template data hash with the hash algorithm. Mimi Dmitry Kasatkin (10): crypto: provide single place for hash algo information keys: change asymmetric keys to use common hash definitions ima: provide support for arbitrary hash algorithms ima: read and use signature hash algorithm ima: pass full xattr with the signature ima: use dynamically allocated hash storage ima: provide dedicated hash algo allocation function ima: support arbitrary hash algorithms in ima_calc_buffer_hash ima: ima_calc_boot_agregate must use SHA1 ima: provide hash algo info in the xattr Mimi Zohar (4): ima: differentiate between template hash and file data hash sizes ima: add audit log support for larger hashes ima: add Kconfig default measurement list template ima: enable support for larger default filedata hash algorithms Roberto Sassu (9): ima: pass the file descriptor to ima_add_violation() ima: pass the filename argument up to ima_add_template_entry() ima: define new function ima_alloc_init_template() to API ima: new templates management mechanism ima: define template fields library and new helpers ima: define new template ima-ng and template fields d-ng and n-ng ima: switch to new template management mechanism ima: defer determining the appraisal hash algorithm for 'ima' template ima: define kernel parameter 'ima_template=' to change configured default Documentation/kernel-parameters.txt | 11 +- Documentation/security/00-INDEX | 2 + Documentation/security/IMA-templates.txt | 87 + crypto/Kconfig| 3 + crypto/Makefile | 1 + crypto/asymmetric_keys/Kconfig| 1 + crypto/asymmetric_keys/public_key.c | 12 -- crypto/asymmetric_keys/rsa.c | 14 +- crypto/asymmetric_keys/x509_cert_parser.c | 12 +- crypto/asymmetric_keys/x509_public_key.c | 6 +- crypto/hash_info.c| 56 ++ include/crypto/hash_info.h| 40 include/crypto/public_key.h | 18 +- include/uapi/linux/hash_info.h| 37 kernel/module_signing.c | 8 +- security/integrity/digsig.c | 5 +- security/integrity/digsig_asymmetric.c| 11 -- security/integrity/evm/evm_main.c | 4 +- security/integrity/iint.c | 2 + security/integrity/ima/Kconfig| 61 ++ security/integrity/ima/Makefile | 2 +- security/integrity/ima/ima.h | 95 +++-- security/integrity/ima/ima_api.c | 129 security/integrity/ima/ima_appraise.c | 100 -- security/integrity/ima/ima_crypto.c | 134 +++-- security/integrity/ima/ima_fs.c
Re: [PATCH v3 4/9] ACPI, x86: Extended error log driver for x86 platform
On Mon, Oct 21, 2013 at 12:03 PM, Luck, Tony wrote: > So this is on top of the 9 patch series (using the V4 that Chen Gong > posted for part 4/9 and V3 for all the others). Obviously it should > be folded back into the series if we go this way. > > It's a bit simplistic right now - the registered function just returns > NOTIFY_DONE in all cases so it will not disturb processing by any other > registered functions - we can make it smarter later. I folded that back into the series. Also switched out the test on whether to print the "No further action is required" message to only do so for corrected errors. Cleaned up some of the commit messages, The result is sitting at: git://git.kernel.org/pub/scm/linux/kernel/git/ras/ras.git eMCA Anything we missed? -Tony -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
pull request: bluetooth-next 2013-10-21
Hi John, One more big pull request for 3.13. These are the patches we queued during last week. Here you will find a lot of improvements to the HCI and L2CAP and MGMT layers with the main ones being a better debugfs support and end of work of splitting L2CAP into Core and Socket parts. Please pull! Gustavo --- The following changes since commit 4b836f393bd8ed111857a6ee1865e44627266ec6: Bluetooth: Read current IAC LAP on controller setup (2013-10-14 19:31:18 -0300) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next for-upstream for you to fetch changes up to d78a32a8fcf775111ccc9ba611a08ca5c29784b6: Bluetooth: Remove sk member from struct l2cap_chan (2013-10-21 13:50:56 -0700) Gustavo Padovan (14): Bluetooth: Extend state_change() call to report errors too Bluetooth: Add l2cap_state_change_and_error() Bluetooth: Access sk_sndtimeo indirectly in l2cap_core.c Bluetooth: Add chan->ops->set_shutdown() Bluetooth: Move l2cap_wait_ack() to l2cap_sock.c Bluetooth: use l2cap_chan_ready() instead of duplicate code Bluetooth: Remove not used struct sock Bluetooth: Do not access chan->sk directly Bluetooth: Hold socket in defer callback in L2CAP socket Bluetooth: Remove socket lock from l2cap_state_change() Bluetooth: Remove parent socket usage from l2cap_core.c Bluetooth: Add L2CAP channel to skb private data Bluetooth: Use bt_cb(skb)->chan to send raw data back Bluetooth: Remove sk member from struct l2cap_chan Johan Hedberg (20): Bluetooth: Fix L2CAP "Command Reject: Invalid CID" response Bluetooth: Remove unused command reject mapping for EMSGSIZE Bluetooth: Remove useless l2cap_err_to_reason function Bluetooth: Ignore A2MP data on non-BR/EDR links Bluetooth: Ignore SMP data on non-LE links Bluetooth: Fix updating the right variable in update_scan_rsp_data() Bluetooth: Reintroduce socket restrictions for LE sockets Bluetooth: Convert auto accept timer to use delayed work Bluetooth: Convert idle timer to use delayed work Bluetooth: Fix ATT socket backwards compatibility with user space Bluetooth: Check for flag instead of features in update_scan_rsp_data() Bluetooth: Check for flag instead of features in update_adv_data() Bluetooth: Add missing check for BREDR_ENABLED flag in update_class() Bluetooth: Refactor set_connectable settings update to separate function Bluetooth: Fix updating settings when there are no HCI commands to send Bluetooth: Move mgmt_pending_find to avoid forward declarations Bluetooth: Fix sending write_scan_enable when BR/EDR is disabled Bluetooth: Move HCI_LIMITED_DISCOVERABLE changes to a general place Bluetooth: Update Set Discoverable to support LE Bluetooth: Fix enabling fast connectable on LE-only controllers Marcel Holtmann (71): Bluetooth: Fix minor coding style issue in set_connectable() Bluetooth: Use hci_request for discoverable timeout handling Bluetooth: Update advertising data based on management commands Bluetooth: Introduce flag for limited discoverable mode Bluetooth: Make mgmt_discoverable() return void Bluetooth: Make mgmt_connectable() return void Bluetooth: Make mgmt_write_scan_failed() return void Bluetooth: Update class of device after changing discoverable mode Bluetooth: Move arming of discoverable timeout to complete handler Bluetooth: Simplify the code for re-arming discoverable timeout Bluetooth: Add HCI command structure for writing current IAC LAP Bluetooth: Add support for entering limited discoverable mode Bluetooth: Make mgmt_new_link_key() return void Bluetooth: Move eir_append_data() function into mgmt.c Bluetooth: Move eir_get_length() function into hci_event.c Bluetooth: Update class of device on discoverable timeout Bluetooth: Add l2cap_chan_no_resume stub for A2MP Bluetooth: Make mgmt_pin_code_request() return void Bluetooth: Make mgmt_pin_code_reply_complete() return void Bluetooth: Make mgmt_pin_code_neg_reply_complete() return void Bluetooth: Make mgmt_auth_failed() return void Bluetooth: Make mgmt_auth_enable_complete() return void Bluetooth: Make mgmt_ssp_enable_complete() return void Bluetooth: Make mgmt_set_class_of_dev_complete() return void Bluetooth: Make mgmt_set_local_name_complete() return void Bluetooth: Make mgmt_read_local_oob_data_reply_complete() return void Bluetooth: Make mgmt_new_ltk() return void Bluetooth: Rename create_ad into create_adv_data Bluetooth: Store scan response data in HCI device Bluetooth: Set the scan response data when needed Bluetooth: Store device name in scan response data
[RFC v2 0/6] DRM revoke support
Hi This is the 2nd revision of reliable unplug support for DRM. As revoke support for the generic VFS layer is still not even close to being merged, this fixes the generic DRM layer to handle unplugged devices gracefully. This series fixes the DRM core to track any running f_ops. During device unplugging (which is the same as revoke()) we mark a device as unplugged so no new f_ops are started. Then we wait for running f_ops to finish and then close all open files (and leave just dummies behind). Now we can unregister the device without waiting for userspace to call close() on all fds. Any comments are welcome. Drivers which provide their own f_ops need to be fixed, other than that this is already working. Tested with udl locally. Running applications get poll(HUP) or SIGBUS for mmaps depending on what they're doing if they access the device after unplugging. Thanks David David Herrmann (6): percpu_rw_semaphore: export symbols for modules percpu_rw_semaphore: add percpu_down_read_trylock() drm: split drm_release() drm: make dev->unplugged reliable drm: make drm_dev_unregister() immediate drm: zap mmaps for dead devices drivers/gpu/drm/Kconfig | 1 + drivers/gpu/drm/drm_drv.c | 5 +- drivers/gpu/drm/drm_fops.c | 111 ++--- drivers/gpu/drm/drm_gem.c | 10 +- drivers/gpu/drm/drm_stub.c | 192 ++-- drivers/gpu/drm/drm_vm.c| 3 +- drivers/gpu/drm/udl/udl_connector.c | 2 +- drivers/gpu/drm/udl/udl_drv.c | 2 +- drivers/gpu/drm/udl/udl_fb.c| 4 +- include/drm/drmP.h | 27 +++-- include/linux/percpu-rwsem.h| 1 + lib/Makefile| 2 +- lib/percpu-rwsem.c | 27 + 13 files changed, 296 insertions(+), 91 deletions(-) -- 1.8.4.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] cpufreq: intel_pstate: fix possible integer overflow
On Monday, October 21, 2013 08:56:22 AM Dirk Brandewie wrote: > On 10/19/2013 08:31 PM, Geyslan G. Bem wrote: > > The expression 'pstate << 8' is evaluated using 32-bit arithmetic while > > 'val' expects an expression of type u64. > > > > Signed-off-by: Geyslan G. Bem > Acked-by: Dirk Brandewie Actually, isn't (pstate << 8) guaranteed not to overflow? > > --- > > drivers/cpufreq/intel_pstate.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c > > index badf620..43446b5 100644 > > --- a/drivers/cpufreq/intel_pstate.c > > +++ b/drivers/cpufreq/intel_pstate.c > > @@ -395,7 +395,7 @@ static void intel_pstate_set_pstate(struct cpudata > > *cpu, int pstate) > > trace_cpu_frequency(pstate * 10, cpu->cpu); > > > > cpu->pstate.current_pstate = pstate; > > - val = pstate << 8; > > + val = (u64)pstate << 8; > > if (limits.no_turbo) > > val |= (u64)1 << 32; > > > > > -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC v2 2/6] percpu_rw_semaphore: add percpu_down_read_trylock()
This is basically a copy of percpu_down_read() but does not sleep if a writer is already active. Same semantics as classic down_read_trylock(). Signed-off-by: David Herrmann --- include/linux/percpu-rwsem.h | 1 + lib/percpu-rwsem.c | 20 2 files changed, 21 insertions(+) diff --git a/include/linux/percpu-rwsem.h b/include/linux/percpu-rwsem.h index 3e88c9a..2f752bd 100644 --- a/include/linux/percpu-rwsem.h +++ b/include/linux/percpu-rwsem.h @@ -16,6 +16,7 @@ struct percpu_rw_semaphore { }; extern void percpu_down_read(struct percpu_rw_semaphore *); +extern int percpu_down_read_trylock(struct percpu_rw_semaphore *); extern void percpu_up_read(struct percpu_rw_semaphore *); extern void percpu_down_write(struct percpu_rw_semaphore *); diff --git a/lib/percpu-rwsem.c b/lib/percpu-rwsem.c index 893586c..596f44b 100644 --- a/lib/percpu-rwsem.c +++ b/lib/percpu-rwsem.c @@ -92,6 +92,26 @@ void percpu_down_read(struct percpu_rw_semaphore *brw) } EXPORT_SYMBOL(percpu_down_read); +int percpu_down_read_trylock(struct percpu_rw_semaphore *brw) +{ + int r; + + if (likely(update_fast_ctr(brw, +1))) { + rwsem_acquire_read(>rw_sem.dep_map, 0, 0, _RET_IP_); + return 1; + } + + r = down_read_trylock(>rw_sem); + if (r) { + atomic_inc(>slow_read_ctr); + /* avoid up_read()->rwsem_release() */ + __up_read(>rw_sem); + } + + return r; +} +EXPORT_SYMBOL(percpu_down_read_trylock); + void percpu_up_read(struct percpu_rw_semaphore *brw) { rwsem_release(>rw_sem.dep_map, 1, _RET_IP_); -- 1.8.4.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC v2 4/6] drm: make dev->unplugged reliable
If we unplug a DRM device, we currently set dev->unplugged and then wait for all open-files to close. Beside rather subtle race-conditions, the main disadvantage is that most DRM device resources will stay allocated and registered. This means the device cannot be re-used by another driver until all user-space processes close()ed their DRM fds. This patch changes the way we track pending fops. Instead of checking for dev->unplugged before each fop, we now call drm_dev_get_active(). This includes a test for dev->unplugged but also marks the device as active. Once the fop is done, we call drm_dev_put_active() and the active-count is decreased. This allows us at _any_ time to see whether there is any running fop. Furthermore, we can mark the device as unplugged and wait for these to finish. So instead of waiting for all users to close() their fds, it is now enough to wait for all running fops to finish. No new fops can be started as the drm_dev_get_active() call fails once the device is unplugged. If no DRM fop sleeps for an indefinite amount of time, this effectively allows us to unregister and free DRM device resource _before_ all DRM fds are closed. Thus preventing user-space from delaying resource destruction. However, this patch doesn't enforce this, yet. Instead, we only introduce the fops tracking. The new drm_dev_shutdown() helper can be used to do a synchronous shutdown. However, this isn't enforced on hotplug-capable drivers, yet. Instead, all drivers using drm_unplug_dev() will still wait for all fds to close before destroying the device (to not break possibly buggy existing drivers). However, for all other drivers we now forcibly wait for all fops to finish during drm_dev_unregister(). (for hotplug-capable drivers this is a no-op as all fds are already closed, anyway). We used to panic if a driver is unloaded while there're active users so this is just a protection against _some_ of the existing races. To allow proper unplugging of _any_ driver, some more protections will follow. Note: The use of percpu_rw_semaphore is an implementation detail. The same effect could be achieved with a spinlock+atomic_t+wait_queue. The percpu_rw_sempahore is only slightly heavier but gives us much better performance and is a lot more convenient to use. Note2: This only fixes the global DRM fops to use drm_dev_get_active(). For drivers to allows full hotplugging, all their level-1 fops also need to be fixed. This can be done separately for each driver, though. Signed-off-by: David Herrmann --- drivers/gpu/drm/Kconfig | 1 + drivers/gpu/drm/drm_drv.c | 5 +- drivers/gpu/drm/drm_fops.c | 25 +-- drivers/gpu/drm/drm_gem.c | 10 +-- drivers/gpu/drm/drm_stub.c | 136 +--- drivers/gpu/drm/drm_vm.c| 3 +- drivers/gpu/drm/udl/udl_connector.c | 2 +- drivers/gpu/drm/udl/udl_fb.c| 4 +- include/drm/drmP.h | 25 +++ 9 files changed, 171 insertions(+), 40 deletions(-) diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 95d..0795558 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -11,6 +11,7 @@ menuconfig DRM select I2C select I2C_ALGOBIT select DMA_SHARED_BUFFER + select PERCPU_RWSEM help Kernel-level support for the Direct Rendering Infrastructure (DRI) introduced in XFree86 4.0. If you say Y here, you need to select diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index b55f138..21cd5f5 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -312,7 +312,7 @@ long drm_ioctl(struct file *filp, dev = file_priv->minor->dev; - if (drm_device_is_unplugged(dev)) + if (!drm_dev_get_active(dev)) return -ENODEV; atomic_inc(>ioctl_count); @@ -403,7 +403,10 @@ long drm_ioctl(struct file *filp, if (kdata != stack_kdata) kfree(kdata); + atomic_dec(>ioctl_count); + drm_dev_put_active(dev); + if (retcode) DRM_DEBUG("ret = %d\n", retcode); return retcode; diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c index ecdd647..3884185 100644 --- a/drivers/gpu/drm/drm_fops.c +++ b/drivers/gpu/drm/drm_fops.c @@ -94,7 +94,7 @@ int drm_open(struct inode *inode, struct file *filp) if (!(dev = minor->dev)) return -ENODEV; - if (drm_device_is_unplugged(dev)) + if (!drm_dev_get_active(dev)) return -ENODEV; if (!dev->open_count++) @@ -118,7 +118,9 @@ int drm_open(struct inode *inode, struct file *filp) if (retcode) goto err_undo; } - return 0; + + retcode = 0; + goto out_active; err_undo: mutex_lock(>struct_mutex); @@ -128,6 +130,8 @@ err_undo: dev->dev_mapping = old_mapping;
[RFC v2 6/6] drm: zap mmaps for dead devices
Once a DRM device is unregistered, user-space must not access any existing mmaps, anymore. As we cannot rely on this, we now zap all of them in drm_dev_unregister(). Any driver which wants to support that needs to protect their fault() and mmap() handlers via drm_dev_get_active(), otherwise users can create new mmaps after/during drm_dev_unregister(). Signed-off-by: David Herrmann --- drivers/gpu/drm/drm_stub.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c index e363b72..274a005 100644 --- a/drivers/gpu/drm/drm_stub.c +++ b/drivers/gpu/drm/drm_stub.c @@ -602,6 +602,10 @@ void drm_dev_unregister(struct drm_device *dev) drm_dev_shutdown(dev); + /* zap all memory mappings (drm_global_mutex must not be locked) */ + if (dev->dev_mapping) + unmap_mapping_range(dev->dev_mapping, 0, LLONG_MAX, 1); + /* We cannot hold drm_global_mutex during drm_dev_shutdown() as it might * dead-lock. Hence, there's a small race between drm_dev_shutdown() and * us locking drm_global_mutex which drm_release() might trigger. To fix -- 1.8.4.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RFC v2 5/6] drm: make drm_dev_unregister() immediate
Drivers which support unplugging currently use drm_unplug_dev() which waits for all open files to be closed before unregistering a device. All other drivers just immediately unregister the device if unplugged or unloaded, which results in panics if there're pending open files. This patch implements proper revoke support for DRM devices. We remove drm_unplug_dev() and make drm_put_dev() an alias for drm_dev_unregister(). Instead of plainly unregistering the device, we now mark the device as dead, close all open files and then unregister the device. This guarantees that all pending actions are done, no new actions can happen and the device is properly unregistered and destroyed. As the underlying VFS layer doesn't provide revoke support for us, we need some special workarounds to support it properly. Any open file still has the file->f_private pointer set to their drm_file object. We cannot reset this pointer so we must keep it allocated. Fortunately, any f_op accesses ((drm_file*)file->f_private)->minor->dev and checks whether the given drm_device is still active before accessing anything else. So what we do during device shutdown is: 1: Mark the device as dead 2: Wait for all pending f_ops to finish 3: Call drm_close() for all open files to fake a close() 4: Only if no (dead) open-file remains, we free the drm_device This procedure guarantees that any pending open-file is now dead but not freed. So any further f_op will return ENODEV as drm_dev_get_active() will fail. Any following real close() call will notice that the device is dead and skip the call to drm_close(). Instead, it simply frees the drm_file private data and decreases the open-count. Once the open-count drops to 0 and the device is already unplugged, the last drm_release() will free the DRM-device. Signed-off-by: David Herrmann --- drivers/gpu/drm/drm_fops.c| 29 +- drivers/gpu/drm/drm_stub.c| 68 ++- drivers/gpu/drm/udl/udl_drv.c | 2 +- include/drm/drmP.h| 1 - 4 files changed, 65 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c index 3884185..c860376 100644 --- a/drivers/gpu/drm/drm_fops.c +++ b/drivers/gpu/drm/drm_fops.c @@ -585,22 +585,37 @@ int drm_release(struct inode *inode, struct file *filp) { struct drm_file *file_priv = filp->private_data; struct drm_device *dev = file_priv->minor->dev; + bool active, alive; mutex_lock(_global_mutex); - drm_close(file_priv); + /* The additional test against dev->filelist is required as +* drm_dev_unregister() cannot hold drm_global_mutex while locking +* dev->active. So if we're called in the short window between +* dev->unplugged being set but drm_global_mutex not yet locked in +* drm_dev_unregister(), we notice it as dev->filelist cannot be empty. +* We treat it as if the device was still active as drm_dev_unregister() +* will wait for us to drop drm_global_mutex, anyway. */ + active = drm_dev_get_active(dev); + alive = active || !list_empty(>filelist); + + if (alive) + drm_close(file_priv); kfree(file_priv); if (!--dev->open_count) { - if (atomic_read(>ioctl_count)) - DRM_ERROR("Device busy: %d\n", - atomic_read(>ioctl_count)); - else + /* If the device is still alive, only call last-close. If it is +* already unregistered (!alive), then free the device as we're +* the last user. */ + if (alive) drm_lastclose(dev); - if (drm_dev_is_unplugged(dev)) - drm_put_dev(dev); + else + drm_dev_free(dev); } + if (active) + drm_dev_put_active(dev); + mutex_unlock(_global_mutex); return 0; diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c index 9218f17..e363b72 100644 --- a/drivers/gpu/drm/drm_stub.c +++ b/drivers/gpu/drm/drm_stub.c @@ -407,30 +407,9 @@ void drm_put_dev(struct drm_device *dev) } drm_dev_unregister(dev); - drm_dev_free(dev); } EXPORT_SYMBOL(drm_put_dev); -void drm_unplug_dev(struct drm_device *dev) -{ - /* for a USB device */ - if (drm_core_check_feature(dev, DRIVER_MODESET)) - drm_unplug_minor(dev->control); - if (dev->render) - drm_unplug_minor(dev->render); - drm_unplug_minor(dev->primary); - - /* TODO: We should call drm_dev_shutdown() here. But to protect against -* buggy drivers, we don't do any synchronous shutdown and instead wait -* for users to go away. */ - dev->unplugged = true; - mb(); - - if (dev->open_count == 0) - drm_put_dev(dev); -}
[RFC v2 3/6] drm: split drm_release()
This splits off real file-destruction into drm_close(). drm_release() now calls drm_close() and then takes care of any device-cleanup which isn't strictly related to file-destruction. Besides making the code more readable, this is needed if we want to close open-files during GPU unplug. We could simply fake a drm_close() on each open-file now to logically close them. Signed-off-by: David Herrmann --- drivers/gpu/drm/drm_fops.c | 65 +++--- include/drm/drmP.h | 1 + 2 files changed, 39 insertions(+), 27 deletions(-) diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c index d0e2766..ecdd647 100644 --- a/drivers/gpu/drm/drm_fops.c +++ b/drivers/gpu/drm/drm_fops.c @@ -450,34 +450,26 @@ int drm_lastclose(struct drm_device * dev) } /** - * Release file. + * drm_close - Close open DRM file + * @file_priv: Open DRM-file to close * - * \param inode device inode - * \param file_priv DRM file private. - * \return zero on success or a negative number on failure. + * This does the real work of drm_release(). All allocated data is freed and + * locks are released. drm_global_mutex must be locked by the caller. The + * drm_file object is not freed and @file_priv->minor->dev stays valid. * - * If the hardware lock is held then free it, and take it again for the kernel - * context since it's necessary to reclaim buffers. Unlink the file private - * data from its list and free it. Decreases the open count and if it reaches - * zero calls drm_lastclose(). + * This does not call drm_lastclose() or any device-cleanup helpers. It's the + * callers responsibility to do this. Drivers must not call this directly. Use + * drm_release() instead. */ -int drm_release(struct inode *inode, struct file *filp) +void drm_close(struct drm_file *file_priv) { - struct drm_file *file_priv = filp->private_data; struct drm_device *dev = file_priv->minor->dev; - int retcode = 0; - - mutex_lock(_global_mutex); DRM_DEBUG("open_count = %d\n", dev->open_count); if (dev->driver->preclose) dev->driver->preclose(dev, file_priv); - /* -* Begin inline drm_release -*/ - DRM_DEBUG("pid = %d, device = 0x%lx, open_count = %d\n", task_pid_nr(current), (long)old_encode_dev(file_priv->minor->device), @@ -490,7 +482,7 @@ int drm_release(struct inode *inode, struct file *filp) /* if the master has gone away we can't do anything with the lock */ if (file_priv->minor->master) - drm_master_release(dev, filp); + drm_master_release(dev, file_priv->filp); if (drm_core_check_feature(dev, DRIVER_HAVE_DMA)) drm_core_reclaim_buffers(dev, file_priv); @@ -573,25 +565,44 @@ int drm_release(struct inode *inode, struct file *filp) drm_prime_destroy_file_private(_priv->prime); put_pid(file_priv->pid); - kfree(file_priv); +} - /* -* End inline drm_release -*/ +/** + * drm_release - Release open DRM file + * @inode: device inode + * @file_priv: DRM file + * + * Release an open DRM file. This is the default callback for DRM fops. + * We free all internally allocated data, release held locks and unlink the + * open-file from the DRM device. If this is the last user of the unplugged + * device, we also release the device. + * + * RETURNS: + * Returns always zero. + */ +int drm_release(struct inode *inode, struct file *filp) +{ + struct drm_file *file_priv = filp->private_data; + struct drm_device *dev = file_priv->minor->dev; + + mutex_lock(_global_mutex); + + drm_close(file_priv); + kfree(file_priv); if (!--dev->open_count) { - if (atomic_read(>ioctl_count)) { + if (atomic_read(>ioctl_count)) DRM_ERROR("Device busy: %d\n", atomic_read(>ioctl_count)); - retcode = -EBUSY; - } else - retcode = drm_lastclose(dev); + else + drm_lastclose(dev); if (drm_device_is_unplugged(dev)) drm_put_dev(dev); } + mutex_unlock(_global_mutex); - return retcode; + return 0; } EXPORT_SYMBOL(drm_release); diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 19b8082..16ff7c4 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -1268,6 +1268,7 @@ extern int drm_open(struct inode *inode, struct file *filp); extern int drm_stub_open(struct inode *inode, struct file *filp); extern ssize_t drm_read(struct file *filp, char __user *buffer, size_t count, loff_t *offset); +extern void drm_close(struct drm_file *file_priv); extern int
[RFC v2 1/6] percpu_rw_semaphore: export symbols for modules
DRM could make great use of percpu-rwsems to track active users and wait for them during hw hotplugging. Export symbols and allow using them in runtime loadable modules. Signed-off-by: David Herrmann --- lib/Makefile | 2 +- lib/percpu-rwsem.c | 7 +++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/Makefile b/lib/Makefile index f3bb2cb..c1572be 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -45,7 +45,7 @@ obj-$(CONFIG_DEBUG_LOCKING_API_SELFTESTS) += locking-selftest.o obj-$(CONFIG_DEBUG_SPINLOCK) += spinlock_debug.o lib-$(CONFIG_RWSEM_GENERIC_SPINLOCK) += rwsem-spinlock.o lib-$(CONFIG_RWSEM_XCHGADD_ALGORITHM) += rwsem.o -lib-$(CONFIG_PERCPU_RWSEM) += percpu-rwsem.o +obj-$(CONFIG_PERCPU_RWSEM) += percpu-rwsem.o CFLAGS_hweight.o = $(subst $(quote),,$(CONFIG_ARCH_HWEIGHT_CFLAGS)) obj-$(CONFIG_GENERIC_HWEIGHT) += hweight.o diff --git a/lib/percpu-rwsem.c b/lib/percpu-rwsem.c index 652a8ee..893586c 100644 --- a/lib/percpu-rwsem.c +++ b/lib/percpu-rwsem.c @@ -7,6 +7,7 @@ #include #include #include +#include int __percpu_init_rwsem(struct percpu_rw_semaphore *brw, const char *name, struct lock_class_key *rwsem_key) @@ -22,12 +23,14 @@ int __percpu_init_rwsem(struct percpu_rw_semaphore *brw, init_waitqueue_head(>write_waitq); return 0; } +EXPORT_SYMBOL(__percpu_init_rwsem); void percpu_free_rwsem(struct percpu_rw_semaphore *brw) { free_percpu(brw->fast_read_ctr); brw->fast_read_ctr = NULL; /* catch use after free bugs */ } +EXPORT_SYMBOL(percpu_free_rwsem); /* * This is the fast-path for down_read/up_read, it only needs to ensure @@ -87,6 +90,7 @@ void percpu_down_read(struct percpu_rw_semaphore *brw) /* avoid up_read()->rwsem_release() */ __up_read(>rw_sem); } +EXPORT_SYMBOL(percpu_down_read); void percpu_up_read(struct percpu_rw_semaphore *brw) { @@ -99,6 +103,7 @@ void percpu_up_read(struct percpu_rw_semaphore *brw) if (atomic_dec_and_test(>slow_read_ctr)) wake_up_all(>write_waitq); } +EXPORT_SYMBOL(percpu_up_read); static int clear_fast_ctr(struct percpu_rw_semaphore *brw) { @@ -150,6 +155,7 @@ void percpu_down_write(struct percpu_rw_semaphore *brw) /* wait for all readers to complete their percpu_up_read() */ wait_event(brw->write_waitq, !atomic_read(>slow_read_ctr)); } +EXPORT_SYMBOL(percpu_down_write); void percpu_up_write(struct percpu_rw_semaphore *brw) { @@ -163,3 +169,4 @@ void percpu_up_write(struct percpu_rw_semaphore *brw) /* the last writer unblocks update_fast_ctr() */ atomic_dec(>write_ctr); } +EXPORT_SYMBOL(percpu_up_write); -- 1.8.4.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] cpufreq: acpi: Add comment under ACPI_ADR_SPACE_SYSTEM_IO case
On Monday, October 21, 2013 12:43:32 PM Viresh Kumar wrote: > On 13 October 2013 06:07, Viresh Kumar wrote: > > On 13/10/2013, Rafael J. Wysocki wrote: > >> -> What about this: > >> > >> /* > >>* The core will not set policy->cur, because cpufreq_driver->get is > >> NULL, > >>* so we need to set it here. However, we have to guess it, because > >> the > >>* current speed is unknown and not detectable via IO ports. > >>*/ > > > > Far better. Attached again. Thanks. > > Have you missed applying this one? Attached again.. I don't think I've missed it. It should be commit 1bab64d in my tree. Thanks! -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] firmware, fix request_firmware_nowait() freeze with no uevent
On 10/21/2013 08:24 AM, Ming Lei wrote: > On Mon, Oct 21, 2013 at 5:35 AM, Prarit Bhargava wrote: >> If request_firmware_nowait() is called with uevent == NULL, the firmware >> completion is never marked complete resulting in a hang in the process. >> >> If uevent is undefined, that means we're not waiting on anything and the >> process should just clean up and complete. While we're at it, add a >> debug dev_dbg() to indicate that the FW has not been found. >> >> Signed-off-by: Prarit Bhargava >> Cc: x...@kernel.org >> Cc: herrmann.der.u...@googlemail.com >> Cc: ming@canonical.com >> Cc: tig...@aivazian.fsnet.co.uk >> --- >> drivers/base/firmware_class.c |6 +- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c >> index 10a4467..95778dc 100644 >> --- a/drivers/base/firmware_class.c >> +++ b/drivers/base/firmware_class.c >> @@ -335,7 +335,8 @@ static bool fw_get_filesystem_firmware(struct device >> *device, >> set_bit(FW_STATUS_DONE, >status); >> complete_all(>completion); >> mutex_unlock(_lock); >> - } >> + } else >> + dev_dbg(device, "firmware: %s not found\n", buf->fw_id); >> >> return success; >> } >> @@ -886,6 +887,9 @@ static int _request_firmware_load(struct firmware_priv >> *fw_priv, bool uevent, >> schedule_delayed_work(_priv->timeout_work, >> timeout); >> >> kobject_uevent(_priv->dev.kobj, KOBJ_ADD); >> + } else { >> + /* if there is no uevent then just cleanup */ >> + schedule_delayed_work(_priv->timeout_work, 0); >> } > > This may not a good idea and might break current NOHOTPLUG > users, Ming, The code is broken for all callers of request_firmware_nowait() with NOHOTPLUG and CONFIG_FW_LOADER_USER_HELPER=y. AFAICT with the two existing cases of this usage in the kernel, both are broken and both are attempting to do the same thing that I'm doing in the x86 microcode ATM. This is the situation as I understand it and please correct me if I'm wrong about the execution path. If I call request_firmware_nowait() with NOHOTPLUG I am essentially saying that there is no uevent associated with this firmware load; that is uevent = 0. request_firmware_work_func() is called as scheduled task, which results in a call to _request_firmware(). _request_firmware() first calls _request_firmware_prepare() which eventually results in a call to __allocate_fw_buf() which does an init_completion(>completion). Returning back up the stack to _request_firmware() we eventually call fw_get_filesystem_firmware(). _If the firmware does not exist_ success is false and the if (success) loop is not executed, and it is important to note that the complete_all(>completion) is _not_ called. fw_get_filesystem_firmware() returns an error so that fw_load_from_user_helper() is called from _request_firmware(). fw_load_from_user_helper() eventually calls _request_firmware_load() and this is where we get into a problem. fw_load_from_user_helper() calls all the file creation, etc., and then hits this chunk of code: if (uevent) { dev_set_uevent_suppress(f_dev, false); dev_dbg(f_dev, "firmware: requesting %s\n", buf->fw_id); if (timeout != MAX_SCHEDULE_TIMEOUT) schedule_delayed_work(_priv->timeout_work, timeout); kobject_uevent(_priv->dev.kobj, KOBJ_ADD); } wait_for_completion(>completion); As I previously said, we've been called with NOHOTPLUG, ie) uevent = 0. That means we skip down to the wait_for_completion(>completion) ... and we wait ... forever. I can reproduce this by using a Dell PE 1850 & the dell_rbu module by doing the following: insmod dell_rbu.ko echo init > /sys/devices/platform/dell_rbu/image_type lsmod | grep dell_rbu (after an hour) [root@dell-pe1850-04 dell_rbu]# lsmod | grep dell_rbu dell_rbu 14315 1 [root@dell-pe1850-04 dell_rbu]# ^^^ that use count is left because the thread is waiting with an existing module ref count. For kicks I put a printk in the dell_rbu code or instrument the _request_firmware() code and did a reboot. Since the completions are finished on system shutdown, I see the code continue to execute at the end of boot. > and how can you make sure the user space application can > complete the request during the timeout time? I see that your question really comes down to "are there additional synchronizations needed in the two drivers that already call the code this way?" I realize that the answer to that is yes and I'll fix those up in a v2. It should be trivial to make those changes AFAICT. I've introduced some additional synchronization via a completion in the x86 microcode and will likely have to do something similar in the other drivers ... although it may be easier to just have the