single maintainer profile directory (was Re: [Ksummit-discuss] [PATCH] media: add a subsystem profile documentation)
On Fri, Sep 13, 2019 at 01:19:21PM -0300, Mauro Carvalho Chehab wrote: > Document the basic policies of the media subsystem profile. > > Signed-off-by: Mauro Carvalho Chehab > --- > > That's basically a modified version of: > https://patchwork.linuxtv.org/patch/52999/ > > Applied to the new template > > Documentation/media/index.rst | 1 + > .../media/maintainer-entry-profile.rst| 140 ++ One idea I proposed to Dan at the Maintainer's Summit was to collect all the profiles is a single directory in Documentation/, since there are two ways someone would want to read profiles: 1) a single profile, based on a MAINTAINERS entry which includes the path 2) all of them, to study for various reasons The #2 case is helped by having them all in one directory with a single index.rst, etc. Then similar profiles are able to merge, etc. -- Kees Cook
Re: single maintainer profile directory (was Re: [Ksummit-discuss] [PATCH] media: add a subsystem profile documentation)
On Tue, Sep 17, 2019 at 10:28:17AM -0300, Mauro Carvalho Chehab wrote:
> No matter where the profiles will physically be stored, its contents belong
> to subsystem-specific documentation, and should be visible at the same index
> file as the kAPI docs is located, as anyone interested on submitting patches
> for a subsystem should be aware about the subsystem specific policies and
> details.
That's a good point. I think your other suggestions below address my
"find them all" case...
> So, my vote is to store them at Documentation/*/ (together
> with the kAPI book).
>
> > since there are
> > two ways someone would want to read profiles:
> >
> > 1) a single profile, based on a MAINTAINERS entry which includes the path
>
> That is the common case. The problem is that the MAINTAINERS file
> currently doesn't generate html output. This is not a problem for
> "old school" devs, but a newbie wanting to collaborate to a single
> specific subsystem may not notice - or understand - the importance
> of the MAINTAINERS file[1].
>
> [1] btw, that's why I submitted a RFC, several years ago, a patch
> converting it to ReST - and later - another patch that would be parsing
> its contents and producing a ReST output.
>
> That's, by far, the most relevant usecase for the profiles, as the
> audience is the ~4k Kernel developers.
Oh yes, having a .rst of the MAINTAINERS file would be pretty great.
Seems like it could just be a build target (and then dependency) for the
sphinx targets?
> > 2) all of them, to study for various reasons
>
> I suspect that only core people will have interest on study them.
>
> Those people are more skilled, and can easily find all those files with
> a simple grep:
>
> $ grep "^P:\s" MAINTAINERS|cut -d':' -f2-
>
> or
>
> $ git grep "^P:\s" MAINTAINERS|cut -d: -f3-
>
> If, for whatever reason, he would prefer an HTML output [1], he could even
> create an index file with all of those with something like:
>
> $ for i in $(grep "^P:\s" MAINTAINERS|cut -d':' -f2-); do
> j=${i/rst/html}; echo "$j"; done
> >Documentation/output/index_profiles.html
>
> We might, instead, teach the Documentation/Makefile to create something
> like the above, but, IMHO, that would just add more complexity to the
> build without a good reason.
>
> [1] I doubt that core devs would prefer seeing this in html form, but some
> variant of the above could be used, for example, to create symlinks for
> all profile docs into a "study" directory.
>
> > The #2 case is helped by having them all in one directory with a single
> > index.rst, etc. Then similar profiles are able to merge, etc.
Whatever the case, please don't let me distract from the actual content
of these profiles: I think it's awesome to capture these details and
makes my life so much easier. :)
--
Kees Cook
Re: [Ksummit-discuss] single maintainer profile directory (was Re: [PATCH] media: add a subsystem profile documentation)
On Wed, Sep 18, 2019 at 08:23:26AM -0300, Mauro Carvalho Chehab wrote: > You can't simply rename MAINTAINERS to .rst and let Sphinx handle it, Right! Sorry, I meant what you'd suggested earlier: having a script convert it at doc-build-time. > The latest RFC about that with I sent was this one: > > > https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1238576.html Cool, I've found it on Lore now too: https://lore.kernel.org/lkml/20160923120733.546a4...@vento.lan/ I think having a script generate the entire thing might be better (instead of having duplicated instructions at the top of both files), which I see is what Joe suggested too. > +.. include:: ../../MAINTAINERS > + :literal: Nah, let's do a full make target as you'd suggested back in that thread. I'll give it a shot if you don't beat me to it. :) -- Kees Cook
Re: [Ksummit-discuss] single maintainer profile directory (was Re: [PATCH] media: add a subsystem profile documentation)
On Sat, Sep 21, 2019 at 01:13:07PM -0600, Jonathan Corbet wrote: > Also, that file is nearly 18K lines long. If some unsuspecting person > generates a PDF and prints it, they're going to get something along the > lines of 300 pages of MAINTAINERS, which may not quite be what they had > in mind. It costs (almost) nothing to put that into HTML output, but > other formats could be painful. Is this something that can be specifically excluded from the non-HTML outputs? (Or rather, specifically included in only the HTML output?) I don't see a way to do that exactly... maybe in my RFC only the html target would get the "real" file? -- Kees Cook
Re: [Ksummit-discuss] [PATCH] media: add a subsystem profile documentation
On Wed, Sep 25, 2019 at 10:13:37AM -0700, Joe Perches wrote:
> On Thu, 2019-09-19 at 09:56 +0300, Dan Carpenter wrote:
> > When I sent a patch, I use get_maintainer.pl then I add whoever the
> > wrote the commit from the Fixes tag. Then I remove Colin King and Kees
> > Cook from the CC list because they worked all over the tree and I know
> > them. I also normally remove LKML if there is another mailing list but
> > at least one subsystem uses LKML for patchwork so this isn't safe.
> >
> > So the safest instructions are "Use get_matainer.pl and add the person
> > who wrote the commit in the Fixes tag".
>
> Maybe add this:
>
> Add the signers of any commit referenced in a "Fixes: " line
> of a patch description.
Oh yes please! I've always done this manually, so that's a nice bit of
automation. :)
>
> ---
> scripts/get_maintainer.pl | 38 +-
> 1 file changed, 37 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
> index 5ef59214c555..34085d146fa2 100755
> --- a/scripts/get_maintainer.pl
> +++ b/scripts/get_maintainer.pl
> @@ -26,6 +26,7 @@ my $email = 1;
> my $email_usename = 1;
> my $email_maintainer = 1;
> my $email_reviewer = 1;
> +my $email_fixes = 1;
> my $email_list = 1;
> my $email_moderated_list = 1;
> my $email_subscriber_list = 0;
> @@ -249,6 +250,7 @@ if (!GetOptions(
> 'r!' => \$email_reviewer,
> 'n!' => \$email_usename,
> 'l!' => \$email_list,
> + 'fixes!' => \$email_fixes,
> 'moderated!' => \$email_moderated_list,
> 's!' => \$email_subscriber_list,
> 'multiline!' => \$output_multiline,
> @@ -503,6 +505,7 @@ sub read_mailmap {
> ## use the filenames on the command line or find the filenames in the
> patchfiles
>
> my @files = ();
> +my @fixes = (); # If a patch description includes
> Fixes: lines
> my @range = ();
> my @keyword_tvi = ();
> my @file_emails = ();
> @@ -568,6 +571,8 @@ foreach my $file (@ARGV) {
> my $filename2 = $2;
> push(@files, $filename1);
> push(@files, $filename2);
> + } elsif (m/^Fixes:\s+([0-9a-fA-F]{6,40})/) {
Is "6" a safe lower bound here? I thought 12 was the way to go?
$ git log | egrep 'Fixes: [a-f0-9]{1,40}' | col2 | awk '{print length }' | sort
| uniq -c | sort -n | tail
238 8
300 7
330 14
344 6
352 11
408 40
425 10
735 16
1866 13
31446 12
Hmpf, 6 is pretty high up there...
> + push(@fixes, $1) if ($email_fixes);
> } elsif (m/^\+\+\+\s+(\S+)/ or m/^---\s+(\S+)/) {
> my $filename = $1;
> $filename =~ s@^[^/]*/@@;
> @@ -598,6 +603,7 @@ foreach my $file (@ARGV) {
> }
>
> @file_emails = uniq(@file_emails);
> +@fixes = uniq(@fixes);
>
> my %email_hash_name;
> my %email_hash_address;
> @@ -612,7 +618,6 @@ my %deduplicate_name_hash = ();
> my %deduplicate_address_hash = ();
>
> my @maintainers = get_maintainers();
> -
> if (@maintainers) {
> @maintainers = merge_email(@maintainers);
> output(@maintainers);
> @@ -927,6 +932,10 @@ sub get_maintainers {
> }
> }
>
> +foreach my $fix (@fixes) {
> + vcs_add_commit_signers($fix, "blamed_fixes");
> +}
> +
> foreach my $email (@email_to, @list_to) {
> $email->[0] = deduplicate_email($email->[0]);
> }
> @@ -1031,6 +1040,7 @@ MAINTAINER field selection options:
> --roles => show roles (status:subsystem, git-signer, list, etc...)
> --rolestats => show roles and statistics (commits/total_commits, %)
> --file-emails => add email addresses found in -f file (default: 0 (off))
> +--fixes => for patches, add signatures of commits with 'Fixes: '
> (default: 1 (on))
Should "Tested-by" and "Co-developed-by" get added to @signature_tags ?
>--scm => print SCM tree(s) if any
>--status => print status if any
>--subsystem => print subsystem name if any
> @@ -1730,6 +1740,32 @@ sub vcs_is_hg {
> return $vcs_used == 2;
> }
>
> +sub vcs_add_commit_signers {
> +return if (!vcs_exists());
> +
> +my ($commit, $desc) = @_;
> +my $commit_count = 0;
> +my $commit_authors_ref;
> +my $commit_signers_ref;
> +my $stats_ref;
> +my @commit_authors = ();
> +my @commit_signers = ();
> +my $cmd;
&g
Re: [Ksummit-discuss] [PATCH] media: add a subsystem profile documentation
On Thu, Sep 26, 2019 at 08:14:03AM -0700, Joe Perches wrote:
> On Wed, 2019-09-25 at 11:40 -0700, Kees Cook wrote:
> > Is "6" a safe lower bound here? I thought 12 was the way to go?
> []
> > $ git log | egrep 'Fixes: [a-f0-9]{1,40}' | col2 | awk '{print length }' |
> > sort | uniq -c | sort -n | tail
> > 238 8
> > 300 7
> > 330 14
> > 344 6
> > 352 11
> > 408 40
> > 425 10
> > 735 16
> >1866 13
> > 31446 12
> >
> > Hmpf, 6 is pretty high up there...
>
> Yes, but your grep then col2 isn't right.
> You are counting all the 'Fixes: commit ' output
> as 6 because that's the length of 'commit'.
the [a-f0-9]{1,40} already excludes "commit".
> I also think the length of the hex commit value doesn't
> matter much as it's got to be a specific single commit
> SHA1 anyway, otherwise the commit id lookup will fail.
Fail enough. We do already have 6-digit SHA1 collisions, so it seemed
like using more than 6 would be nicer? *shrug* I don't have a strong
opinion. :)
>
> > > > @@ -1031,6 +1040,7 @@ MAINTAINER field selection options:
> > > --roles => show roles (status:subsystem, git-signer, list, etc...)
> > > --rolestats => show roles and statistics (commits/total_commits, %)
> > > --file-emails => add email addresses found in -f file (default: 0
> > > (off))
> > > +--fixes => for patches, add signatures of commits with 'Fixes:
> > > ' (default: 1 (on))
> >
> > Should "Tested-by" and "Co-developed-by" get added to @signature_tags ?
>
> All "-by:" signatures are added.
Ah, I'd missed where that happened. I do note that's only when
git-all-signature-types is set, which is default 0. (/me goes to add
this to his invocations...)
my $email_git_all_signature_types = 0;
...
if ($email_git_all_signature_types) {
$signature_pattern = "(.+?)[Bb][Yy]:";
} else {
$signature_pattern = "\(" . join("|", @signature_tags) . "\)";
}
> > @commit_authors is unused?
>
> Yes, authors are already required to sign-off so
> it's just duplicating already existing signatures.
Sure, it just seemed odd to populate it if it wasn't going to be used.
--
Kees Cook
Re: [Ksummit-discuss] [PATCH] media: add a subsystem profile documentation
On Thu, Sep 26, 2019 at 09:02:07AM -0700, Joe Perches wrote:
> > the [a-f0-9]{1,40} already excludes "commit".
>
> No it doesn't as commit starts with c which matches [a-f0-9]{1,40}
Whoops! Yes, sorry, you're right. I needed a trailing whitespace in the
regex.
--
Kees Cook
Re: [PATCH] media: technisat-usb2: break out of loop at end of buffer
On Wed, Jul 03, 2019 at 03:52:39PM +0100, Sean Young wrote:
> Ensure we do not access the buffer beyond the end if no 0xff byte
> is encountered.
>
> Reported-by: syzbot+eaaaf38a95427be88...@syzkaller.appspotmail.com
> Signed-off-by: Sean Young
Both you and Phong Tran appear to be working on fixing this. At a
glance, this patch appears to be more complete in that it makes the code
flow more sane too.
Reviewed-by: Kees Cook
-Kees
> ---
> drivers/media/usb/dvb-usb/technisat-usb2.c | 22 ++
> 1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/media/usb/dvb-usb/technisat-usb2.c
> b/drivers/media/usb/dvb-usb/technisat-usb2.c
> index c659e18b358b..676d233d46d5 100644
> --- a/drivers/media/usb/dvb-usb/technisat-usb2.c
> +++ b/drivers/media/usb/dvb-usb/technisat-usb2.c
> @@ -608,10 +608,9 @@ static int technisat_usb2_frontend_attach(struct
> dvb_usb_adapter *a)
> static int technisat_usb2_get_ir(struct dvb_usb_device *d)
> {
> struct technisat_usb2_state *state = d->priv;
> - u8 *buf = state->buf;
> - u8 *b;
> - int ret;
> struct ir_raw_event ev;
> + u8 *buf = state->buf;
> + int i, ret;
>
> buf[0] = GET_IR_DATA_VENDOR_REQUEST;
> buf[1] = 0x08;
> @@ -647,26 +646,25 @@ static int technisat_usb2_get_ir(struct dvb_usb_device
> *d)
> return 0; /* no key pressed */
>
> /* decoding */
> - b = buf+1;
>
> #if 0
> deb_rc("RC: %d ", ret);
> - debug_dump(b, ret, deb_rc);
> + debug_dump(buf + 1, ret, deb_rc);
> #endif
>
> ev.pulse = 0;
> - while (1) {
> - ev.pulse = !ev.pulse;
> - ev.duration = (*b * FIRMWARE_CLOCK_DIVISOR *
> FIRMWARE_CLOCK_TICK) / 1000;
> - ir_raw_event_store(d->rc_dev, &ev);
> -
> - b++;
> - if (*b == 0xff) {
> + for (i = 1; i < ARRAY_SIZE(state->buf); i++) {
> + if (buf[i] == 0xff) {
> ev.pulse = 0;
> ev.duration = 88*2;
> ir_raw_event_store(d->rc_dev, &ev);
> break;
> }
> +
> + ev.pulse = !ev.pulse;
> + ev.duration = (buf[i] * FIRMWARE_CLOCK_DIVISOR *
> +FIRMWARE_CLOCK_TICK) / 1000;
> + ir_raw_event_store(d->rc_dev, &ev);
> }
>
> ir_raw_event_handle(d->rc_dev);
> --
> 2.21.0
>
--
Kees Cook
Re: [PATCH 6/7] media: don't do an unsigned int with a 31 bit shift
On Thu, Aug 22, 2019 at 04:39:33PM -0300, Mauro Carvalho Chehab wrote: > Doing something like: > > i32 foo = 1, bar; > > bar = foo << 31; > > has an undefined behavior in C, as warned by cppcheck, as we're > shifting a signed integer. > > Instead, force the numbers to be unsigned, in order to solve this > issue. I also recommend using the BIT() macro, which does the ULing correctly, etc. i.e. instead of: - keyup = (gpio & ir->mask_keyup) ? 1 << 31 : 0; + keyup = (gpio & ir->mask_keyup) ? 1UL << 31 : 0; use: - keyup = (gpio & ir->mask_keyup) ? 1 << 31 : 0; + keyup = (gpio & ir->mask_keyup) ? BIT(31) : 0; -- Kees Cook
Re: [PATCH] solo6x10: use designated initializers
On Mon, Dec 19, 2016 at 11:56 AM, Andrey Utkin
wrote:
> On Fri, Dec 16, 2016 at 05:05:36PM -0800, Kees Cook wrote:
>> Prepare to mark sensitive kernel structures for randomization by making
>> sure they're using designated initializers. These were identified during
>> allyesconfig builds of x86, arm, and arm64, with most initializer fixes
>> extracted from grsecurity.
>
> Ok I've reviewed all the patchset, googled a bit and now I see what's
> going on.
>
>>
>> Signed-off-by: Kees Cook
>> ---
>> drivers/media/pci/solo6x10/solo6x10-g723.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/pci/solo6x10/solo6x10-g723.c
>> b/drivers/media/pci/solo6x10/solo6x10-g723.c
>> index 6a35107aca25..36e93540bb49 100644
>> --- a/drivers/media/pci/solo6x10/solo6x10-g723.c
>> +++ b/drivers/media/pci/solo6x10/solo6x10-g723.c
>> @@ -350,7 +350,7 @@ static int solo_snd_pcm_init(struct solo_dev *solo_dev)
>>
>> int solo_g723_init(struct solo_dev *solo_dev)
>> {
>> - static struct snd_device_ops ops = { NULL };
>> + static struct snd_device_ops ops = { };
>
> I'm not that keen on syntax subtleties, but...
> * Empty initializer is not quite "designated" as I can judge.
> * From brief googling I see that empty initializer is not valid in
>some C standards.
>
> Since `ops` is static, what about this?
> For the variant given below, you have my signoff.
>
>> --- a/drivers/media/pci/solo6x10/solo6x10-g723.c
>> +++ b/drivers/media/pci/solo6x10/solo6x10-g723.c
>> @@ -350,7 +350,7 @@ static int solo_snd_pcm_init(struct solo_dev *solo_dev)
>>
>> int solo_g723_init(struct solo_dev *solo_dev)
>> {
>> - static struct snd_device_ops ops = { NULL };
>> + static struct snd_device_ops ops;
Ah! Yes, thanks. That works fine too. :) Can this be const as well?
-Kees
--
Kees Cook
Nexus Security
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] dvb: fix potential format string leak
Make sure that a format string cannot accidentally leak into the printk
buffer.
Signed-off-by: Kees Cook
---
drivers/media/dvb-frontends/dib9000.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/dvb-frontends/dib9000.c
b/drivers/media/dvb-frontends/dib9000.c
index 6201c59..61b2cfe 100644
--- a/drivers/media/dvb-frontends/dib9000.c
+++ b/drivers/media/dvb-frontends/dib9000.c
@@ -649,7 +649,7 @@ static int dib9000_risc_debug_buf(struct dib9000_state
*state, u16 * data, u8 si
b[2 * (size - 2) - 1] = '\0'; /* Bullet proof the buffer */
if (*b == '~') {
b++;
- dprintk(b);
+ dprintk("%s", b);
} else
dprintk("RISC%d: %d.%04d %s", state->fe_id, ts / 1, ts %
1, *b ? b : "");
return 1;
--
1.7.9.5
--
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] mtk-vcodec: use designated initializers
Prepare to mark sensitive kernel structures for randomization by making
sure they're using designated initializers. These were identified during
allyesconfig builds of x86, arm, and arm64, with most initializer fixes
extracted from grsecurity.
Signed-off-by: Kees Cook
---
drivers/media/platform/mtk-vcodec/venc/venc_h264_if.c | 8
drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c | 8
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/media/platform/mtk-vcodec/venc/venc_h264_if.c
b/drivers/media/platform/mtk-vcodec/venc/venc_h264_if.c
index b76c80bdf30b..4eb3be37ba14 100644
--- a/drivers/media/platform/mtk-vcodec/venc/venc_h264_if.c
+++ b/drivers/media/platform/mtk-vcodec/venc/venc_h264_if.c
@@ -665,10 +665,10 @@ static int h264_enc_deinit(unsigned long handle)
}
static const struct venc_common_if venc_h264_if = {
- h264_enc_init,
- h264_enc_encode,
- h264_enc_set_param,
- h264_enc_deinit,
+ .init = h264_enc_init,
+ .encode = h264_enc_encode,
+ .set_param = h264_enc_set_param,
+ .deinit = h264_enc_deinit,
};
const struct venc_common_if *get_h264_enc_comm_if(void);
diff --git a/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c
b/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c
index 544f57186243..a6fa145f2c54 100644
--- a/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c
+++ b/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c
@@ -470,10 +470,10 @@ static int vp8_enc_deinit(unsigned long handle)
}
static const struct venc_common_if venc_vp8_if = {
- vp8_enc_init,
- vp8_enc_encode,
- vp8_enc_set_param,
- vp8_enc_deinit,
+ .init = vp8_enc_init,
+ .encode = vp8_enc_encode,
+ .set_param = vp8_enc_set_param,
+ .deinit = vp8_enc_deinit,
};
const struct venc_common_if *get_vp8_enc_comm_if(void);
--
2.7.4
--
Kees Cook
Nexus Security
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] solo6x10: use designated initializers
Prepare to mark sensitive kernel structures for randomization by making
sure they're using designated initializers. These were identified during
allyesconfig builds of x86, arm, and arm64, with most initializer fixes
extracted from grsecurity.
Signed-off-by: Kees Cook
---
drivers/media/pci/solo6x10/solo6x10-g723.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/pci/solo6x10/solo6x10-g723.c
b/drivers/media/pci/solo6x10/solo6x10-g723.c
index 6a35107aca25..36e93540bb49 100644
--- a/drivers/media/pci/solo6x10/solo6x10-g723.c
+++ b/drivers/media/pci/solo6x10/solo6x10-g723.c
@@ -350,7 +350,7 @@ static int solo_snd_pcm_init(struct solo_dev *solo_dev)
int solo_g723_init(struct solo_dev *solo_dev)
{
- static struct snd_device_ops ops = { NULL };
+ static struct snd_device_ops ops = { };
struct snd_card *card;
struct snd_kcontrol_new kctl;
char name[32];
--
2.7.4
--
Kees Cook
Nexus Security
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] subsystem:linux-media CVE-2016-5400
On Fri, Jul 15, 2016 at 8:40 AM, James Patrick-Evans wrote:
> This patch addresses CVE-2016-5400, a local DOS vulnerability caused by a
> memory leak in the airspy usb device driver. The vulnerability is triggered
> when more than 64 usb devices register with v4l2 of type VFL_TYPE_SDR or
> VFL_TYPE_SUBDEV.A badusb device can emulate 64 of these devices then through
> continual emulated connect/disconnect of the 65th device, cause the kernel
> to run out of RAM and crash the kernel. The vulnerability exists in kernel
> versions from 3.17 to current 4.7.
> The memory leak is caused by the probe function of the airspy driver
> mishandeling errors and not freeing the corresponding control structures
> when an error occours registering the device to v4l2 core.
Thanks for getting this fixed!
> Signed-off-by: James Patrick-Evans
Reviewed-by: Kees Cook
> ---
> drivers/media/usb/airspy/airspy.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/usb/airspy/airspy.c
> b/drivers/media/usb/airspy/airspy.c
> index 87c1293..6c3ac8b 100644
> --- a/drivers/media/usb/airspy/airspy.c
> +++ b/drivers/media/usb/airspy/airspy.c
> @@ -1072,7 +1072,7 @@ static int airspy_probe(struct usb_interface *intf,
> if (ret) {
> dev_err(s->dev, "Failed to register as video device (%d)\n",
> ret);
> - goto err_unregister_v4l2_dev;
> + goto err_free_controls;
> }
> dev_info(s->dev, "Registered as %s\n",
> video_device_node_name(&s->vdev));
> --
> 1.9.1
>
-Kees
--
Kees Cook
Brillo & Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] subsystem:linux-media CVE-2016-5400
[fixing Mauro's email...]
On Fri, Jul 15, 2016 at 11:52 AM, Kees Cook wrote:
> On Fri, Jul 15, 2016 at 8:40 AM, James Patrick-Evans wrote:
>> This patch addresses CVE-2016-5400, a local DOS vulnerability caused by a
>> memory leak in the airspy usb device driver. The vulnerability is triggered
>> when more than 64 usb devices register with v4l2 of type VFL_TYPE_SDR or
>> VFL_TYPE_SUBDEV.A badusb device can emulate 64 of these devices then through
>> continual emulated connect/disconnect of the 65th device, cause the kernel
>> to run out of RAM and crash the kernel. The vulnerability exists in kernel
>> versions from 3.17 to current 4.7.
>> The memory leak is caused by the probe function of the airspy driver
>> mishandeling errors and not freeing the corresponding control structures
>> when an error occours registering the device to v4l2 core.
>
> Thanks for getting this fixed!
>
>> Signed-off-by: James Patrick-Evans
>
> Reviewed-by: Kees Cook
>
>> ---
>> drivers/media/usb/airspy/airspy.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/usb/airspy/airspy.c
>> b/drivers/media/usb/airspy/airspy.c
>> index 87c1293..6c3ac8b 100644
>> --- a/drivers/media/usb/airspy/airspy.c
>> +++ b/drivers/media/usb/airspy/airspy.c
>> @@ -1072,7 +1072,7 @@ static int airspy_probe(struct usb_interface *intf,
>> if (ret) {
>> dev_err(s->dev, "Failed to register as video device (%d)\n",
>> ret);
>> - goto err_unregister_v4l2_dev;
>> + goto err_free_controls;
>> }
>> dev_info(s->dev, "Registered as %s\n",
>> video_device_node_name(&s->vdev));
>> --
>> 1.9.1
>>
>
> -Kees
>
> --
> Kees Cook
> Brillo & Chrome OS Security
--
Kees Cook
Brillo & Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0435/1285] Replace numeric parameter like 0444 with macro
There are so many of these, I wonder if it'd be better to just do one
giant patch, or at least break them up by subsystem instead of by
individual source file...
-Kees
On Tue, Aug 2, 2016 at 4:11 AM, Baole Ni wrote:
> I find that the developers often just specified the numeric value
> when calling a macro which is defined with a parameter for access permission.
> As we know, these numeric value for access permission have had the
> corresponding macro,
> and that using macro can improve the robustness and readability of the code,
> thus, I suggest replacing the numeric parameter with the macro.
>
> Signed-off-by: Chuansheng Liu
> Signed-off-by: Baole Ni
> ---
> drivers/media/i2c/tvp5150.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> index 0b6d46c..d8ffd88 100644
> --- a/drivers/media/i2c/tvp5150.c
> +++ b/drivers/media/i2c/tvp5150.c
> @@ -33,7 +33,7 @@ MODULE_LICENSE("GPL");
>
>
> static int debug;
> -module_param(debug, int, 0644);
> +module_param(debug, int, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
> MODULE_PARM_DESC(debug, "Debug level (0-2)");
>
> struct tvp5150 {
> --
> 2.9.2
>
--
Kees Cook
Chrome OS & Brillo Security
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] drm, video: fix use-before-NULL-check
Fix potential crashes due to use-before-NULL situations.
Signed-off-by: Kees Cook
---
drivers/gpu/drm/drm_fb_helper.c |3 ++-
drivers/media/video/em28xx/em28xx-video.c |3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index de82e20..8dd7e6f 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -94,10 +94,11 @@ static bool
drm_fb_helper_connector_parse_command_line(struct drm_fb_helper_conn
int i;
enum drm_connector_force force = DRM_FORCE_UNSPECIFIED;
struct drm_fb_helper_cmdline_mode *cmdline_mode;
- struct drm_connector *connector = fb_helper_conn->connector;
+ struct drm_connector *connector;
if (!fb_helper_conn)
return false;
+ connector = fb_helper_conn->connector;
cmdline_mode = &fb_helper_conn->cmdline_mode;
if (!mode_option)
diff --git a/drivers/media/video/em28xx/em28xx-video.c
b/drivers/media/video/em28xx/em28xx-video.c
index 7b9ec6e..95a4b60 100644
--- a/drivers/media/video/em28xx/em28xx-video.c
+++ b/drivers/media/video/em28xx/em28xx-video.c
@@ -277,12 +277,13 @@ static void em28xx_copy_vbi(struct em28xx *dev,
{
void *startwrite, *startread;
int offset;
- int bytesperline = dev->vbi_width;
+ int bytesperline;
if (dev == NULL) {
em28xx_isocdbg("dev is null\n");
return;
}
+ bytesperline = dev->vbi_width;
if (dma_q == NULL) {
em28xx_isocdbg("dma_q is null\n");
--
1.7.1
--
Kees Cook
Ubuntu Security Team
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] video: fix potential use-before-NULL-check crash
Moves use to after NULL-check.
Signed-off-by: Kees Cook
---
Sent before as part of https://patchwork.kernel.org/patch/138711/ but it
still hasn't been applied.
---
drivers/media/video/em28xx/em28xx-video.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/media/video/em28xx/em28xx-video.c
b/drivers/media/video/em28xx/em28xx-video.c
index 7b9ec6e..95a4b60 100644
--- a/drivers/media/video/em28xx/em28xx-video.c
+++ b/drivers/media/video/em28xx/em28xx-video.c
@@ -277,12 +277,13 @@ static void em28xx_copy_vbi(struct em28xx *dev,
{
void *startwrite, *startread;
int offset;
- int bytesperline = dev->vbi_width;
+ int bytesperline;
if (dev == NULL) {
em28xx_isocdbg("dev is null\n");
return;
}
+ bytesperline = dev->vbi_width;
if (dma_q == NULL) {
em28xx_isocdbg("dma_q is null\n");
--
1.7.1
--
Kees Cook
Ubuntu Security Team
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] doc: no singing
Stop that, stop that! You're not going to do a song while I'm here. Signed-off-by: Kees Cook --- https://lkml.org/lkml/2013/12/4/786 http://www.youtube.com/watch?v=g3YiPC91QUk#t=62 --- Documentation/cgroups/resource_counter.txt |2 +- Documentation/video4linux/si476x.txt |2 +- arch/score/lib/checksum.S |2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/cgroups/resource_counter.txt b/Documentation/cgroups/resource_counter.txt index c4d99ed0b418..caa6d662b230 100644 --- a/Documentation/cgroups/resource_counter.txt +++ b/Documentation/cgroups/resource_counter.txt @@ -95,7 +95,7 @@ to work with it. f. u64 res_counter_uncharge_until (struct res_counter *rc, struct res_counter *top, -unsinged long val) +unsigned long val) Almost same as res_cunter_uncharge() but propagation of uncharge stops when rc == top. This is useful when kill a res_coutner in diff --git a/Documentation/video4linux/si476x.txt b/Documentation/video4linux/si476x.txt index 2f9b4875ab8a..616607955aaf 100644 --- a/Documentation/video4linux/si476x.txt +++ b/Documentation/video4linux/si476x.txt @@ -147,7 +147,7 @@ The drivers exposes following files: 0x12 | readfreq | Current tuned frequency - 0x14 | freqoff | Singed frequency offset in units of + 0x14 | freqoff | Signed frequency offset in units of | | 2ppm 0x15 | rssi | Signed value of RSSI in dBuV diff --git a/arch/score/lib/checksum.S b/arch/score/lib/checksum.S index 706157edc7d5..1141f2b4a501 100644 --- a/arch/score/lib/checksum.S +++ b/arch/score/lib/checksum.S @@ -137,7 +137,7 @@ ENTRY(csum_partial) ldi r25, 0 mv r10, r5 cmpi.c r5, 0x8 - blt small_csumcpy /* < 8(singed) bytes to copy */ + blt small_csumcpy /* < 8(signed) bytes to copy */ cmpi.c r5, 0x0 beq out andri.c r25, src, 0x1 /* odd buffer? */ -- 1.7.9.5 -- Kees Cook Chrome OS Security -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC 00/15] Zero ****s, hugload of hugs <3
On Fri, Nov 30, 2018 at 11:27 AM Jarkko Sakkinen wrote: > > In order to comply with the CoC, replace with a hug. Heh. I support the replacement of the stronger language, but I find "hug", "hugged", and "hugging" to be very weird replacements. Can we bikeshed this to "heck", "hecked", and "hecking" (or "heckin" to follow true Doggo meme style). "This API is hugged" doesn't make any sense to me. "This API is hecked" is better, or at least funnier (to me). "Hug this interface" similarly makes no sense, but "Heck this interface" seems better. "Don't touch my hecking code", "What the heck were they thinking?" etc... "hug" is odd. Better yet, since it's only 17 files, how about doing context-specific changes? "This API is terrible", "Hateful interface", "Don't touch my freakin' code", "What in the world were they thinking?" etc? -Kees > > Jarkko Sakkinen (15): > MIPS: replace with a hug > Documentation: replace with a hug > drm/nouveau: replace with a hug > m68k: replace with a hug > parisc: replace with a hug > cpufreq: replace with a hug > ide: replace with a hug > media: replace with a hug > mtd: replace with a hug > net/sunhme: replace with a hug > scsi: replace with a hug > inotify: replace with a hug > irq: replace with a hug > lib: replace with a hug > net: replace with a hug > > Documentation/kernel-hacking/locking.rst | 2 +- > arch/m68k/include/asm/sun3ints.h | 2 +- > arch/mips/pci/ops-bridge.c| 24 +-- > arch/mips/sgi-ip22/ip22-setup.c | 2 +- > arch/parisc/kernel/sys_parisc.c | 2 +- > drivers/cpufreq/powernow-k7.c | 2 +- > .../gpu/drm/nouveau/nvkm/subdev/bios/init.c | 2 +- > .../nouveau/nvkm/subdev/pmu/fuc/macros.fuc| 2 +- > drivers/ide/cmd640.c | 2 +- > drivers/media/i2c/bt819.c | 8 --- > drivers/mtd/mtd_blkdevs.c | 2 +- > drivers/net/ethernet/sun/sunhme.c | 4 ++-- > drivers/scsi/qlogicpti.h | 2 +- > fs/notify/inotify/inotify_user.c | 2 +- > kernel/irq/timings.c | 2 +- > lib/vsprintf.c| 2 +- > net/core/skbuff.c | 2 +- > 17 files changed, 33 insertions(+), 31 deletions(-) > > -- > 2.19.1 > -- Kees Cook
Re: [PATCH 1/3] media: use strscpy() instead of strlcpy()
On Mon, Sep 10, 2018 at 5:19 AM, Mauro Carvalho Chehab wrote: > The implementation of strscpy() is more robust and safer. > > That's now the recommended way to copy NUL terminated strings. This looks fine since I don't see anything using the strlcpy() return value (the return value meaning between strlcpy() and strscpy() differs). Reviewed-by: Kees Cook -Kees -- Kees Cook Pixel Security
Re: [PATCH 2/3] media: replace strcpy() by strscpy()
On Mon, Sep 10, 2018 at 5:19 AM, Mauro Carvalho Chehab
wrote:
> The strcpy() function is being deprecated upstream. Replace
> it by the safer strscpy().
Did you verify that all the destination buffers here are arrays and
not pointers? For example:
struct thing {
char buffer[64];
char *ptr;
}
strscpy(instance->buffer, source, sizeof(instance->buffer));
is correct.
But:
strscpy(instance->ptr, source, sizeof(instance->ptr));
will not be and will truncate strings to sizeof(char *).
If you _did_ verify this, I'd love to know more about your tooling. :)
-Kees
--
Kees Cook
Pixel Security
Re: [PATCH 3/3] media: replace strncpy() by strscpy()
On Mon, Sep 10, 2018 at 5:19 AM, Mauro Carvalho Chehab wrote: > The strncpy() function is being deprecated upstream. Replace > it by the safer strscpy(). This one I'm quite concerned about. This could lead to kernel memory exposures if any of the callers depend on strncpy()'s trailing NUL-padding to clear a buffer of prior contents. How did you validate that for these changes? -Kees -- Kees Cook Pixel Security
Re: [PATCH 3/3] media: replace strncpy() by strscpy()
On Mon, Sep 10, 2018 at 11:34 AM, Mauro Carvalho Chehab wrote: > Em Mon, 10 Sep 2018 09:18:05 -0700 > Kees Cook escreveu: > >> On Mon, Sep 10, 2018 at 5:19 AM, Mauro Carvalho Chehab >> wrote: >> > The strncpy() function is being deprecated upstream. Replace >> > it by the safer strscpy(). >> >> This one I'm quite concerned about. This could lead to kernel memory >> exposures if any of the callers depend on strncpy()'s trailing >> NUL-padding to clear a buffer of prior contents. >> >> How did you validate that for these changes? > > That's actually easy for those familiar with the V4L2 API. There are > several fields at either uAPI or kAPI (or both) that have strings. > > For example, a video input has a name. > > So, for one familiar with the V4L2 API, it is clear that something > like: > > + strscpy(inp->name, zr->card.input[inp->index].name, > + sizeof(inp->name)); > > Is just filling the uAPI with the name of Input, with is, typically, > something like: > S-Video > Television > Radio > Composite > > A visual inspection of the patch shows that, on almost all cases, it is > either filling a device driver's name (used mainly for debug routines), > a video Input, a format description string, or the video caps fields > name and driver. It looks like the ioctl path also pre-clears the output buffer before handing it over to the per-driver routines, so I think this looks okay. It's a large patch, but if you're comfortable with it, go for it. :) -Kees -- Kees Cook Pixel Security
[PATCH 17/31] media/i2c/tc358743: Initialize timer
This converts to use setup_timer() to set callback and data, though it
doesn't look like this would have worked with timer checking enabled
since no init_timer() was ever called before.
Cc: Mats Randgaard
Cc: Mauro Carvalho Chehab
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook
---
drivers/media/i2c/tc358743.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index 5788af238b86..94e722e0f4e0 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -1964,8 +1964,8 @@ static int tc358743_probe(struct i2c_client *client,
} else {
INIT_WORK(&state->work_i2c_poll,
tc358743_work_i2c_poll);
- state->timer.data = (unsigned long)state;
- state->timer.function = tc358743_irq_poll_timer;
+ setup_timer(&state->timer, tc358743_irq_poll_timer,
+ (unsigned long)state);
state->timer.expires = jiffies +
msecs_to_jiffies(POLL_INTERVAL_MS);
add_timer(&state->timer);
--
2.7.4
[PATCH v2 17/31] media/i2c/tc358743: Initialize timer
This converts to use setup_timer() to set callback and data, though it
doesn't look like this would have worked with timer checking enabled
since no init_timer() was ever called before.
Cc: Mats Randgaard
Cc: Mauro Carvalho Chehab
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook
---
drivers/media/i2c/tc358743.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index e6f5c363ccab..07ad6a3ff1ec 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -1964,8 +1964,8 @@ static int tc358743_probe(struct i2c_client *client,
} else {
INIT_WORK(&state->work_i2c_poll,
tc358743_work_i2c_poll);
- state->timer.data = (unsigned long)state;
- state->timer.function = tc358743_irq_poll_timer;
+ setup_timer(&state->timer, tc358743_irq_poll_timer,
+ (unsigned long)state);
state->timer.expires = jiffies +
msecs_to_jiffies(POLL_INTERVAL_MS);
add_timer(&state->timer);
--
2.7.4
[PATCH] input: Convert timers to use timer_setup()
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.
One input_dev user hijacks the input_dev software autorepeat timer to
perform its own repeat management. However, there is not path back
to the existing status variable, so add a generic one to the input
structure and use that instead.
Cc: Dmitry Torokhov
Cc: Mauro Carvalho Chehab
Cc: Hans Verkuil
Cc: Sakari Ailus
Cc: Geliang Tang
Cc: linux-in...@vger.kernel.org
Cc: linux-media@vger.kernel.org
Cc: Thomas Gleixner
Signed-off-by: Kees Cook
---
This requires commit 686fef928bba ("timer: Prepare to change timer
callback argument type") in v4.14-rc3, but should be otherwise
stand-alone.
---
drivers/input/input.c | 12 ++--
drivers/media/pci/ttpci/av7110.h| 1 -
drivers/media/pci/ttpci/av7110_ir.c | 16
include/linux/input.h | 2 ++
4 files changed, 16 insertions(+), 15 deletions(-)
diff --git a/drivers/input/input.c b/drivers/input/input.c
index d268fdc23c64..497ad2dcb699 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -76,7 +76,7 @@ static void input_start_autorepeat(struct input_dev *dev, int
code)
{
if (test_bit(EV_REP, dev->evbit) &&
dev->rep[REP_PERIOD] && dev->rep[REP_DELAY] &&
- dev->timer.data) {
+ dev->timer.function) {
dev->repeat_key = code;
mod_timer(&dev->timer,
jiffies + msecs_to_jiffies(dev->rep[REP_DELAY]));
@@ -179,9 +179,9 @@ static void input_pass_event(struct input_dev *dev,
* dev->event_lock here to avoid racing with input_event
* which may cause keys get "stuck".
*/
-static void input_repeat_key(unsigned long data)
+static void input_repeat_key(struct timer_list *t)
{
- struct input_dev *dev = (void *) data;
+ struct input_dev *dev = from_timer(dev, t, timer);
unsigned long flags;
spin_lock_irqsave(&dev->event_lock, flags);
@@ -1790,7 +1790,7 @@ struct input_dev *input_allocate_device(void)
device_initialize(&dev->dev);
mutex_init(&dev->mutex);
spin_lock_init(&dev->event_lock);
- init_timer(&dev->timer);
+ timer_setup(&dev->timer, NULL, 0);
INIT_LIST_HEAD(&dev->h_list);
INIT_LIST_HEAD(&dev->node);
@@ -2053,8 +2053,8 @@ static void devm_input_device_unregister(struct device
*dev, void *res)
*/
void input_enable_softrepeat(struct input_dev *dev, int delay, int period)
{
- dev->timer.data = (unsigned long) dev;
- dev->timer.function = input_repeat_key;
+ dev->timer.function = (TIMER_FUNC_TYPE)input_repeat_key;
+ dev->timer_data = 0;
dev->rep[REP_DELAY] = delay;
dev->rep[REP_PERIOD] = period;
}
diff --git a/drivers/media/pci/ttpci/av7110.h b/drivers/media/pci/ttpci/av7110.h
index 347827925c14..b98a4f3006df 100644
--- a/drivers/media/pci/ttpci/av7110.h
+++ b/drivers/media/pci/ttpci/av7110.h
@@ -93,7 +93,6 @@ struct infrared {
u8 inversion;
u16 last_key;
u16 last_toggle;
- u8 delay_timer_finished;
};
diff --git a/drivers/media/pci/ttpci/av7110_ir.c
b/drivers/media/pci/ttpci/av7110_ir.c
index ca05198de2c2..a883caa6488c 100644
--- a/drivers/media/pci/ttpci/av7110_ir.c
+++ b/drivers/media/pci/ttpci/av7110_ir.c
@@ -155,16 +155,16 @@ static void av7110_emit_key(unsigned long parm)
if (timer_pending(&ir->keyup_timer)) {
del_timer(&ir->keyup_timer);
if (ir->last_key != keycode || toggle != ir->last_toggle) {
- ir->delay_timer_finished = 0;
+ ir->input_dev->timer_data = 0;
input_event(ir->input_dev, EV_KEY, ir->last_key, 0);
input_event(ir->input_dev, EV_KEY, keycode, 1);
input_sync(ir->input_dev);
- } else if (ir->delay_timer_finished) {
+ } else if (ir->input_dev->timer_data) {
input_event(ir->input_dev, EV_KEY, keycode, 2);
input_sync(ir->input_dev);
}
} else {
- ir->delay_timer_finished = 0;
+ ir->input_dev->timer_data = 0;
input_event(ir->input_dev, EV_KEY, keycode, 1);
input_sync(ir->input_dev);
}
@@ -206,11 +206,12 @@ static void input_register_keys(struct infrared *ir)
/* called by the input driver after rep[REP_DELAY] ms */
-static void input_repeat_key(unsigned long parm)
+sta
[PATCH] media: saa7146: Convert timers to use timer_setup()
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.
Cc: Hans Verkuil
Cc: Mauro Carvalho Chehab
Cc: linux-media@vger.kernel.org
Cc: Thomas Gleixner
Signed-off-by: Kees Cook
---
This requires commit 686fef928bba ("timer: Prepare to change timer
callback argument type") in v4.14-rc3, but should be otherwise
stand-alone.
---
drivers/media/common/saa7146/saa7146_fops.c | 4 ++--
drivers/media/common/saa7146/saa7146_vbi.c | 3 +--
drivers/media/common/saa7146/saa7146_video.c | 3 +--
include/media/drv-intf/saa7146_vv.h | 2 +-
4 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/media/common/saa7146/saa7146_fops.c
b/drivers/media/common/saa7146/saa7146_fops.c
index c4664f0da874..8c87d6837c49 100644
--- a/drivers/media/common/saa7146/saa7146_fops.c
+++ b/drivers/media/common/saa7146/saa7146_fops.c
@@ -163,9 +163,9 @@ void saa7146_buffer_next(struct saa7146_dev *dev,
}
}
-void saa7146_buffer_timeout(unsigned long data)
+void saa7146_buffer_timeout(struct timer_list *t)
{
- struct saa7146_dmaqueue *q = (struct saa7146_dmaqueue*)data;
+ struct saa7146_dmaqueue *q = from_timer(q, t, timeout);
struct saa7146_dev *dev = q->dev;
unsigned long flags;
diff --git a/drivers/media/common/saa7146/saa7146_vbi.c
b/drivers/media/common/saa7146/saa7146_vbi.c
index f65c0b934c22..0f7599ad4958 100644
--- a/drivers/media/common/saa7146/saa7146_vbi.c
+++ b/drivers/media/common/saa7146/saa7146_vbi.c
@@ -366,8 +366,7 @@ static void vbi_init(struct saa7146_dev *dev, struct
saa7146_vv *vv)
INIT_LIST_HEAD(&vv->vbi_dmaq.queue);
- setup_timer(&vv->vbi_dmaq.timeout, saa7146_buffer_timeout,
- (unsigned long)(&vv->vbi_dmaq));
+ timer_setup(&vv->vbi_dmaq.timeout, saa7146_buffer_timeout, 0);
vv->vbi_dmaq.dev = dev;
init_waitqueue_head(&vv->vbi_wq);
diff --git a/drivers/media/common/saa7146/saa7146_video.c
b/drivers/media/common/saa7146/saa7146_video.c
index 37b4654dc21c..ed9b5300dcce 100644
--- a/drivers/media/common/saa7146/saa7146_video.c
+++ b/drivers/media/common/saa7146/saa7146_video.c
@@ -1201,8 +1201,7 @@ static void video_init(struct saa7146_dev *dev, struct
saa7146_vv *vv)
{
INIT_LIST_HEAD(&vv->video_dmaq.queue);
- setup_timer(&vv->video_dmaq.timeout, saa7146_buffer_timeout,
- (unsigned long)(&vv->video_dmaq));
+ timer_setup(&vv->video_dmaq.timeout, saa7146_buffer_timeout, 0);
vv->video_dmaq.dev = dev;
/* set some default values */
diff --git a/include/media/drv-intf/saa7146_vv.h
b/include/media/drv-intf/saa7146_vv.h
index fbe36d05fb79..b1f25240968f 100644
--- a/include/media/drv-intf/saa7146_vv.h
+++ b/include/media/drv-intf/saa7146_vv.h
@@ -184,7 +184,7 @@ int saa7146_unregister_device(struct video_device *vid,
struct saa7146_dev *dev)
void saa7146_buffer_finish(struct saa7146_dev *dev, struct saa7146_dmaqueue
*q, int state);
void saa7146_buffer_next(struct saa7146_dev *dev, struct saa7146_dmaqueue
*q,int vbi);
int saa7146_buffer_queue(struct saa7146_dev *dev, struct saa7146_dmaqueue *q,
struct saa7146_buf *buf);
-void saa7146_buffer_timeout(unsigned long data);
+void saa7146_buffer_timeout(struct timer_list *t);
void saa7146_dma_free(struct saa7146_dev* dev,struct videobuf_queue *q,
struct saa7146_buf *buf);
--
2.7.4
--
Kees Cook
Pixel Security
[PATCH] media: serial_ir: Convert timers to use timer_setup()
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.
Cc: Sean Young
Cc: Mauro Carvalho Chehab
Cc: linux-media@vger.kernel.org
Cc: Thomas Gleixner
Signed-off-by: Kees Cook
---
This requires commit 686fef928bba ("timer: Prepare to change timer
callback argument type") in v4.14-rc3, but should be otherwise
stand-alone.
---
drivers/media/rc/serial_ir.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/media/rc/serial_ir.c b/drivers/media/rc/serial_ir.c
index 8b66926bc16a..8bf5637b3a69 100644
--- a/drivers/media/rc/serial_ir.c
+++ b/drivers/media/rc/serial_ir.c
@@ -470,7 +470,7 @@ static int hardware_init_port(void)
return 0;
}
-static void serial_ir_timeout(unsigned long arg)
+static void serial_ir_timeout(struct timer_list *unused)
{
DEFINE_IR_RAW_EVENT(ev);
@@ -540,8 +540,7 @@ static int serial_ir_probe(struct platform_device *dev)
serial_ir.rcdev = rcdev;
- setup_timer(&serial_ir.timeout_timer, serial_ir_timeout,
- (unsigned long)&serial_ir);
+ timer_setup(&serial_ir.timeout_timer, serial_ir_timeout, 0);
result = devm_request_irq(&dev->dev, irq, serial_ir_irq_handler,
share_irq ? IRQF_SHARED : 0,
--
2.7.4
--
Kees Cook
Pixel Security
[PATCH] media: saa7134: Convert timers to use timer_setup()
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.
Cc: Mauro Carvalho Chehab
Cc: Sakari Ailus
Cc: Sean Young
Cc: Geliang Tang
Cc: Hans Verkuil
Cc: linux-media@vger.kernel.org
Cc: Thomas Gleixner
Signed-off-by: Kees Cook
---
This requires commit 686fef928bba ("timer: Prepare to change timer
callback argument type") in v4.14-rc3, but should be otherwise
stand-alone.
---
drivers/media/pci/saa7134/saa7134-core.c | 6 +++---
drivers/media/pci/saa7134/saa7134-input.c | 9 -
drivers/media/pci/saa7134/saa7134-ts.c| 3 +--
drivers/media/pci/saa7134/saa7134-vbi.c | 3 +--
drivers/media/pci/saa7134/saa7134-video.c | 3 +--
drivers/media/pci/saa7134/saa7134.h | 2 +-
6 files changed, 11 insertions(+), 15 deletions(-)
diff --git a/drivers/media/pci/saa7134/saa7134-core.c
b/drivers/media/pci/saa7134/saa7134-core.c
index 7976c5a12ca8..9e76de2411ae 100644
--- a/drivers/media/pci/saa7134/saa7134-core.c
+++ b/drivers/media/pci/saa7134/saa7134-core.c
@@ -338,9 +338,9 @@ void saa7134_buffer_next(struct saa7134_dev *dev,
}
}
-void saa7134_buffer_timeout(unsigned long data)
+void saa7134_buffer_timeout(struct timer_list *t)
{
- struct saa7134_dmaqueue *q = (struct saa7134_dmaqueue *)data;
+ struct saa7134_dmaqueue *q = from_timer(q, t, timeout);
struct saa7134_dev *dev = q->dev;
unsigned long flags;
@@ -378,7 +378,7 @@ void saa7134_stop_streaming(struct saa7134_dev *dev, struct
saa7134_dmaqueue *q)
}
}
spin_unlock_irqrestore(&dev->slock, flags);
- saa7134_buffer_timeout((unsigned long)q); /* also calls
del_timer(&q->timeout) */
+ saa7134_buffer_timeout(&q->timeout); /* also calls
del_timer(&q->timeout) */
}
EXPORT_SYMBOL_GPL(saa7134_stop_streaming);
diff --git a/drivers/media/pci/saa7134/saa7134-input.c
b/drivers/media/pci/saa7134/saa7134-input.c
index 9337e4615519..2d5abeddc079 100644
--- a/drivers/media/pci/saa7134/saa7134-input.c
+++ b/drivers/media/pci/saa7134/saa7134-input.c
@@ -447,10 +447,10 @@ void saa7134_input_irq(struct saa7134_dev *dev)
}
}
-static void saa7134_input_timer(unsigned long data)
+static void saa7134_input_timer(struct timer_list *t)
{
- struct saa7134_dev *dev = (struct saa7134_dev *)data;
- struct saa7134_card_ir *ir = dev->remote;
+ struct saa7134_card_ir *ir = from_timer(ir, t, timer);
+ struct saa7134_dev *dev = ir->dev->priv;
build_key(dev);
mod_timer(&ir->timer, jiffies + msecs_to_jiffies(ir->polling));
@@ -507,8 +507,7 @@ static int __saa7134_ir_start(void *priv)
ir->running = true;
if (ir->polling) {
- setup_timer(&ir->timer, saa7134_input_timer,
- (unsigned long)dev);
+ timer_setup(&ir->timer, saa7134_input_timer, 0);
ir->timer.expires = jiffies + HZ;
add_timer(&ir->timer);
}
diff --git a/drivers/media/pci/saa7134/saa7134-ts.c
b/drivers/media/pci/saa7134/saa7134-ts.c
index 7414878af9e0..2be703617e29 100644
--- a/drivers/media/pci/saa7134/saa7134-ts.c
+++ b/drivers/media/pci/saa7134/saa7134-ts.c
@@ -223,8 +223,7 @@ int saa7134_ts_init1(struct saa7134_dev *dev)
dev->ts.nr_packets = ts_nr_packets;
INIT_LIST_HEAD(&dev->ts_q.queue);
- setup_timer(&dev->ts_q.timeout, saa7134_buffer_timeout,
- (unsigned long)(&dev->ts_q));
+ timer_setup(&dev->ts_q.timeout, saa7134_buffer_timeout, 0);
dev->ts_q.dev = dev;
dev->ts_q.need_two = 1;
dev->ts_started= 0;
diff --git a/drivers/media/pci/saa7134/saa7134-vbi.c
b/drivers/media/pci/saa7134/saa7134-vbi.c
index bcad9b2d9bb3..af494c6147f1 100644
--- a/drivers/media/pci/saa7134/saa7134-vbi.c
+++ b/drivers/media/pci/saa7134/saa7134-vbi.c
@@ -181,8 +181,7 @@ struct vb2_ops saa7134_vbi_qops = {
int saa7134_vbi_init1(struct saa7134_dev *dev)
{
INIT_LIST_HEAD(&dev->vbi_q.queue);
- setup_timer(&dev->vbi_q.timeout, saa7134_buffer_timeout,
- (unsigned long)(&dev->vbi_q));
+ timer_setup(&dev->vbi_q.timeout, saa7134_buffer_timeout, 0);
dev->vbi_q.dev = dev;
if (vbibufs < 2)
diff --git a/drivers/media/pci/saa7134/saa7134-video.c
b/drivers/media/pci/saa7134/saa7134-video.c
index 51d42bbf969e..82d2a24644e4 100644
--- a/drivers/media/pci/saa7134/saa7134-video.c
+++ b/drivers/media/pci/saa7134/saa7134-video.c
@@ -2145,8 +2145,7 @@ int saa7134_video_init1(struct saa7134_dev *dev)
dev->automute = 0;
INIT_LIST_HEAD(&dev->video_q.queue);
- setup_timer(&dev->video_q.
[PATCH] media/saa7146: Convert timers to use timer_setup()
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly. This requires adding a pointer to
hold the timer's target file, as there won't be a way to pass this in the
future.
Cc: Hans Verkuil
Cc: Mauro Carvalho Chehab
Cc: linux-media@vger.kernel.org
Cc: Thomas Gleixner
Signed-off-by: Kees Cook
---
This requires commit 686fef928bba ("timer: Prepare to change timer
callback argument type") in v4.14-rc3, but should be otherwise
stand-alone.
---
drivers/media/common/saa7146/saa7146_fops.c | 2 +-
drivers/media/common/saa7146/saa7146_vbi.c | 9 +
include/media/drv-intf/saa7146_vv.h | 1 +
3 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/media/common/saa7146/saa7146_fops.c
b/drivers/media/common/saa7146/saa7146_fops.c
index 930d2c94d5d3..c4664f0da874 100644
--- a/drivers/media/common/saa7146/saa7146_fops.c
+++ b/drivers/media/common/saa7146/saa7146_fops.c
@@ -559,7 +559,7 @@ int saa7146_vv_init(struct saa7146_dev* dev, struct
saa7146_ext_vv *ext_vv)
vbi->start[1] = 312;
vbi->count[1] = 16;
- init_timer(&vv->vbi_read_timeout);
+ timer_setup(&vv->vbi_read_timeout, NULL, 0);
vv->ov_fb.capability = V4L2_FBUF_CAP_LIST_CLIPPING;
vv->ov_fb.flags = V4L2_FBUF_FLAG_PRIMARY;
diff --git a/drivers/media/common/saa7146/saa7146_vbi.c
b/drivers/media/common/saa7146/saa7146_vbi.c
index d79e4d7ecd9f..f65c0b934c22 100644
--- a/drivers/media/common/saa7146/saa7146_vbi.c
+++ b/drivers/media/common/saa7146/saa7146_vbi.c
@@ -348,9 +348,10 @@ static void vbi_stop(struct saa7146_fh *fh, struct file
*file)
spin_unlock_irqrestore(&dev->slock, flags);
}
-static void vbi_read_timeout(unsigned long data)
+static void vbi_read_timeout(struct timer_list *t)
{
- struct file *file = (struct file*)data;
+ struct saa7146_vv *vv = from_timer(vv, t, vbi_read_timeout);
+ struct file *file = vv->vbi_read_timeout_file;
struct saa7146_fh *fh = file->private_data;
struct saa7146_dev *dev = fh->dev;
@@ -401,8 +402,8 @@ static int vbi_open(struct saa7146_dev *dev, struct file
*file)
sizeof(struct saa7146_buf),
file, &dev->v4l2_lock);
- vv->vbi_read_timeout.function = vbi_read_timeout;
- vv->vbi_read_timeout.data = (unsigned long)file;
+ vv->vbi_read_timeout.function = (TIMER_FUNC_TYPE)vbi_read_timeout;
+ vv->vbi_read_timeout_file = file;
/* initialize the brs */
if ( 0 != (SAA7146_USE_PORT_B_FOR_VBI & dev->ext_vv_data->flags)) {
diff --git a/include/media/drv-intf/saa7146_vv.h
b/include/media/drv-intf/saa7146_vv.h
index 0da6ccc0615b..fbe36d05fb79 100644
--- a/include/media/drv-intf/saa7146_vv.h
+++ b/include/media/drv-intf/saa7146_vv.h
@@ -107,6 +107,7 @@ struct saa7146_vv
struct saa7146_dmaqueue vbi_dmaq;
struct v4l2_vbi_format vbi_fmt;
struct timer_list vbi_read_timeout;
+ struct file *vbi_read_timeout_file;
/* vbi workaround interrupt queue */
wait_queue_head_t vbi_wq;
int vbi_fieldcount;
--
2.7.4
--
Kees Cook
Pixel Security
[PATCH] staging/atomisp: Convert timers to use timer_setup()
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.
Cc: Mauro Carvalho Chehab
Cc: Greg Kroah-Hartman
Cc: Alan Cox
Cc: Daeseok Youn
Cc: Arnd Bergmann
Cc: linux-media@vger.kernel.org
Cc: de...@driverdev.osuosl.org
Cc: Thomas Gleixner
Signed-off-by: Kees Cook
---
This requires commit 686fef928bba ("timer: Prepare to change timer
callback argument type") in v4.14-rc3, but should be otherwise
stand-alone.
---
drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c | 13 -
drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.h | 6 +-
.../media/atomisp/pci/atomisp2/atomisp_compat_css20.c | 2 +-
drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c | 15 +--
4 files changed, 11 insertions(+), 25 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
index f48bf451c1f5..6a38cd64b2a1 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
@@ -1660,20 +1660,15 @@ void atomisp_css_flush(struct atomisp_device *isp)
dev_dbg(isp->dev, "atomisp css flush done\n");
}
-#ifndef ISP2401
-void atomisp_wdt(unsigned long isp_addr)
-#else
-void atomisp_wdt(unsigned long pipe_addr)
-#endif
+void atomisp_wdt(struct timer_list *t)
{
#ifndef ISP2401
- struct atomisp_device *isp = (struct atomisp_device *)isp_addr;
+ struct atomisp_sub_device *asd = from_timer(asd, t, wdt);
#else
- struct atomisp_video_pipe *pipe =
- (struct atomisp_video_pipe *)pipe_addr;
+ struct atomisp_video_pipe *pipe = from_timer(pipe, t, wdt);
struct atomisp_sub_device *asd = pipe->asd;
- struct atomisp_device *isp = asd->isp;
#endif
+ struct atomisp_device *isp = asd->isp;
#ifdef ISP2401
atomic_inc(&pipe->wdt_count);
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.h
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.h
index 31ba4e613d13..4bb83864da2e 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.h
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.h
@@ -85,11 +85,7 @@ static inline void __iomem
*atomisp_get_io_virt_addr(unsigned int address)
void atomisp_msi_irq_init(struct atomisp_device *isp, struct pci_dev *dev);
void atomisp_msi_irq_uninit(struct atomisp_device *isp, struct pci_dev *dev);
void atomisp_wdt_work(struct work_struct *work);
-#ifndef ISP2401
-void atomisp_wdt(unsigned long isp_addr);
-#else
-void atomisp_wdt(unsigned long pipe_addr);
-#endif
+void atomisp_wdt(struct timer_list *t);
void atomisp_setup_flash(struct atomisp_sub_device *asd);
irqreturn_t atomisp_isr(int irq, void *dev);
irqreturn_t atomisp_isr_thread(int irq, void *isp_ptr);
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c
index 05897b747349..0b907474e024 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c
@@ -4515,7 +4515,7 @@ int atomisp_css_isr_thread(struct atomisp_device *isp,
for (i = 0; i < isp->num_of_streams; i++)
atomisp_wdt_stop(&isp->asd[i], 0);
#ifndef ISP2401
- atomisp_wdt((unsigned long)isp);
+ atomisp_wdt(&isp->asd[0].wdt);
#else
queue_work(isp->wdt_work_queue, &isp->wdt_work);
#endif
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c
index 663aa916e3ca..76dc779f32c3 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c
@@ -1155,17 +1155,12 @@ static int init_atomisp_wdts(struct atomisp_device *isp)
struct atomisp_sub_device *asd = &isp->asd[i];
asd = &isp->asd[i];
#ifndef ISP2401
- setup_timer(&asd->wdt, atomisp_wdt, (unsigned long)isp);
+ timer_setup(&asd->wdt, atomisp_wdt, 0);
#else
- setup_timer(&asd->video_out_capture.wdt,
- atomisp_wdt, (unsigned long)&asd->video_out_capture);
- setup_timer(&asd->video_out_preview.wdt,
- atomisp_wdt, (unsigned long)&asd->video_out_preview);
- setup_timer(&asd->video_out_vf.wdt,
- atomisp_wdt, (unsigned long)&asd->video_out_vf);
- setup_timer(&asd->video_out_video_capture.wdt,
- atomisp_w
Re: [PATCH][RFC] kernel.h: provide array iterator
On Thu, Mar 15, 2018 at 3:00 AM, Kieran Bingham
wrote:
> Simplify array iteration with a helper to iterate each entry in an array.
> Utilise the existing ARRAY_SIZE macro to identify the length of the array
> and pointer arithmetic to process each item as a for loop.
>
> Signed-off-by: Kieran Bingham
> ---
> include/linux/kernel.h | 10 ++
> 1 file changed, 10 insertions(+)
>
> The use of static arrays to store data is a common use case throughout the
> kernel. Along with that is the obvious need to iterate that data.
>
> In fact there are just shy of 5000 instances of iterating a static array:
> git grep "for .*ARRAY_SIZE" | wc -l
> 4943
I suspect the main question is "Does this macro make the code easier to read?"
I think it does, and we have other kinds of iterators like this in the
kernel already. Would it be worth building a Coccinelle script to do
the 5000 replacements?
-Kees
>
> When working on the UVC driver - I found that I needed to split one such
> iteration into two parts, and at the same time felt that this could be
> refactored to be cleaner / easier to read.
>
> I do however worry that this simple short patch might not be desired or could
> also be heavily bikeshedded due to it's potential wide spread use (though
> perhaps that would be a good thing to have more users) ... but here it is,
> along with an example usage below which is part of a separate series.
>
> The aim is to simplify iteration on static arrays, in the same way that we
> have
> iterators for lists. The use of the ARRAY_SIZE macro, provides all the
> protections given by "__must_be_array(arr)" to this macro too.
>
> Regards
>
> Kieran
>
> =
> Example Usage from a pending UVC development:
>
> +#define for_each_uvc_urb(uvc_urb, uvc_streaming) \
> + for_each_array_element(uvc_urb, uvc_streaming->uvc_urb)
>
> /*
> * Uninitialize isochronous/bulk URBs and free transfer buffers.
> */
> static void uvc_uninit_video(struct uvc_streaming *stream, int free_buffers)
> {
> - struct urb *urb;
> - unsigned int i;
> + struct uvc_urb *uvc_urb;
>
> uvc_video_stats_stop(stream);
>
> - for (i = 0; i < UVC_URBS; ++i) {
> - struct uvc_urb *uvc_urb = &stream->uvc_urb[i];
> + for_each_uvc_urb(uvc_urb, stream)
> + usb_kill_urb(uvc_urb->urb);
>
> - urb = uvc_urb->urb;
> - if (urb == NULL)
> - continue;
> + flush_workqueue(stream->async_wq);
>
> - usb_kill_urb(urb);
> - usb_free_urb(urb);
> + for_each_uvc_urb(uvc_urb, stream) {
> + usb_free_urb(uvc_urb->urb);
> uvc_urb->urb = NULL;
> }
>
> if (free_buffers)
> uvc_free_urb_buffers(stream);
> }
> =
>
>
>
>
> diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> index ce51455e2adf..95d7dae248b7 100644
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -70,6 +70,16 @@
> */
> #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) +
> __must_be_array(arr))
>
> +/**
> + * for_each_array_element - Iterate all items in an array
> + * @elem: pointer of array type for iteration cursor
> + * @array: array to be iterated
> + */
> +#define for_each_array_element(elem, array) \
> + for (elem = &(array)[0]; \
> +elem < &(array)[ARRAY_SIZE(array)]; \
> +++elem)
> +
> #define u64_to_user_ptr(x) ( \
> { \
> typecheck(u64, x); \
> --
> 2.7.4
>
--
Kees Cook
Pixel Security
Re: [PATCH] media: media-device: fix ioctl function types
On Fri, Apr 27, 2018 at 12:54 PM, Sami Tolvanen wrote:
> This change fixes function types for media device ioctls to avoid
> indirect call mismatches with Control-Flow Integrity checking.
>
> Signed-off-by: Sami Tolvanen
Thanks for sending these!
Reviewed-by: Kees Cook
-Kees
> ---
> drivers/media/media-device.c | 21 +++--
> 1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index 35e81f7c0d2f..bc5c024906e6 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -54,9 +54,10 @@ static int media_device_close(struct file *filp)
> return 0;
> }
>
> -static int media_device_get_info(struct media_device *dev,
> -struct media_device_info *info)
> +static long media_device_get_info(struct media_device *dev, void *arg)
> {
> + struct media_device_info *info = (struct media_device_info *)arg;
> +
> memset(info, 0, sizeof(*info));
>
> if (dev->driver_name[0])
> @@ -93,9 +94,9 @@ static struct media_entity *find_entity(struct media_device
> *mdev, u32 id)
> return NULL;
> }
>
> -static long media_device_enum_entities(struct media_device *mdev,
> - struct media_entity_desc *entd)
> +static long media_device_enum_entities(struct media_device *mdev, void *arg)
> {
> + struct media_entity_desc *entd = (struct media_entity_desc *)arg;
> struct media_entity *ent;
>
> ent = find_entity(mdev, entd->id);
> @@ -146,9 +147,9 @@ static void media_device_kpad_to_upad(const struct
> media_pad *kpad,
> upad->flags = kpad->flags;
> }
>
> -static long media_device_enum_links(struct media_device *mdev,
> - struct media_links_enum *links)
> +static long media_device_enum_links(struct media_device *mdev, void *arg)
> {
> + struct media_links_enum *links = (struct media_links_enum *)arg;
> struct media_entity *entity;
>
> entity = find_entity(mdev, links->entity);
> @@ -195,9 +196,9 @@ static long media_device_enum_links(struct media_device
> *mdev,
> return 0;
> }
>
> -static long media_device_setup_link(struct media_device *mdev,
> - struct media_link_desc *linkd)
> +static long media_device_setup_link(struct media_device *mdev, void *arg)
> {
> + struct media_link_desc *linkd = (struct media_link_desc *)arg;
> struct media_link *link = NULL;
> struct media_entity *source;
> struct media_entity *sink;
> @@ -225,9 +226,9 @@ static long media_device_setup_link(struct media_device
> *mdev,
> return __media_entity_setup_link(link, linkd->flags);
> }
>
> -static long media_device_get_topology(struct media_device *mdev,
> - struct media_v2_topology *topo)
> +static long media_device_get_topology(struct media_device *mdev, void *arg)
> {
> + struct media_v2_topology *topo = (struct media_v2_topology *)arg;
> struct media_entity *entity;
> struct media_interface *intf;
> struct media_pad *pad;
> --
> 2.17.0.441.gb46fe60e1d-goog
>
--
Kees Cook
Pixel Security
Re: [PATCH] media: v4l2-ioctl: fix function types for IOCTL_INFO_STD
On Fri, Apr 27, 2018 at 11:59 AM, Sami Tolvanen wrote:
> This change fixes indirect call mismatches with Control-Flow Integrity
> checking, which are caused by calling standard ioctls using a function
> pointer that doesn't match the type of the actual function.
>
> Signed-off-by: Sami Tolvanen
I think this actually makes things much more readable in the end. Thanks!
Reviewed-by: Kees Cook
-Kees
> ---
> drivers/media/v4l2-core/v4l2-ioctl.c | 72 ++--
> 1 file changed, 46 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
> b/drivers/media/v4l2-core/v4l2-ioctl.c
> index f48c505550e0..d50a06ab3509 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -2489,11 +2489,8 @@ struct v4l2_ioctl_info {
> unsigned int ioctl;
> u32 flags;
> const char * const name;
> - union {
> - u32 offset;
> - int (*func)(const struct v4l2_ioctl_ops *ops,
> - struct file *file, void *fh, void *p);
> - } u;
> + int (*func)(const struct v4l2_ioctl_ops *ops, struct file *file,
> + void *fh, void *p);
> void (*debug)(const void *arg, bool write_only);
> };
>
> @@ -2501,27 +2498,24 @@ struct v4l2_ioctl_info {
> #define INFO_FL_PRIO (1 << 0)
> /* This control can be valid if the filehandle passes a control handler. */
> #define INFO_FL_CTRL (1 << 1)
> -/* This is a standard ioctl, no need for special code */
> -#define INFO_FL_STD(1 << 2)
> /* This is ioctl has its own function */
> -#define INFO_FL_FUNC (1 << 3)
> +#define INFO_FL_FUNC (1 << 2)
> /* Queuing ioctl */
> -#define INFO_FL_QUEUE (1 << 4)
> +#define INFO_FL_QUEUE (1 << 3)
> /* Always copy back result, even on error */
> -#define INFO_FL_ALWAYS_COPY(1 << 5)
> +#define INFO_FL_ALWAYS_COPY(1 << 4)
> /* Zero struct from after the field to the end */
> #define INFO_FL_CLEAR(v4l2_struct, field) \
> ((offsetof(struct v4l2_struct, field) + \
> sizeof(((struct v4l2_struct *)0)->field)) << 16)
> #define INFO_FL_CLEAR_MASK (_IOC_SIZEMASK << 16)
>
> -#define IOCTL_INFO_STD(_ioctl, _vidioc, _debug, _flags)
> \
> - [_IOC_NR(_ioctl)] = { \
> - .ioctl = _ioctl,\
> - .flags = _flags | INFO_FL_STD, \
> - .name = #_ioctl,\
> - .u.offset = offsetof(struct v4l2_ioctl_ops, _vidioc), \
> - .debug = _debug,\
> +#define DEFINE_IOCTL_STD_FNC(_vidioc) \
> + static int __v4l_ ## _vidioc ## _fnc( \
> + const struct v4l2_ioctl_ops *ops, \
> + struct file *file, void *fh, void *p) \
> + { \
> + return ops->_vidioc(file, fh, p); \
> }
>
> #define IOCTL_INFO_FNC(_ioctl, _func, _debug, _flags) \
> @@ -2529,10 +2523,42 @@ struct v4l2_ioctl_info {
> .ioctl = _ioctl,\
> .flags = _flags | INFO_FL_FUNC, \
> .name = #_ioctl,\
> - .u.func = _func,\
> + .func = _func, \
> .debug = _debug,\
> }
>
> +#define IOCTL_INFO_STD(_ioctl, _vidioc, _debug, _flags)\
> + IOCTL_INFO_FNC(_ioctl, __v4l_ ## _vidioc ## _fnc, _debug, _flags)
> +
> +DEFINE_IOCTL_STD_FNC(vidioc_g_fbuf)
> +DEFINE_IOCTL_STD_FNC(vidioc_s_fbuf)
> +DEFINE_IOCTL_STD_FNC(vidioc_expbuf)
> +DEFINE_IOCTL_STD_FNC(vidioc_g_std)
> +DEFINE_IOCTL_STD_FNC(vidioc_g_audio)
> +DEFINE_IOCTL_STD_FNC(vidioc_s_audio)
> +DEFINE_IOCTL_STD_FNC(vidioc_g_input)
> +DEFINE_IOCTL_STD_FNC(vidioc_g_edid)
> +DEFINE_IOCTL_STD_FNC(vidioc_s_edid)
> +DEFINE_IOCTL_STD_FNC(vidioc_g_output)
> +DEFINE_IOCTL_STD_FNC(vidioc_g_audout)
> +DEFINE_IOCTL_STD_FNC(vidioc_s_audout)
> +DEFINE_IOCTL_STD_FNC(vidioc_g_jpegcomp)
> +DEFINE_IOCTL_STD_FNC(vidioc_s_jpegcomp)
> +DEFINE_IOCTL_STD_FNC(vidioc_enumaudio)
> +DEFINE_IOCTL_STD_FNC(vidioc_enum
[PATCH] media/saa7146: Convert timers to use timer_setup()
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly. This requires adding a pointer to
hold the timer's target file, as there won't be a way to pass this in the
future.
Cc: Hans Verkuil
Cc: Mauro Carvalho Chehab
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook
---
drivers/media/common/saa7146/saa7146_fops.c | 2 +-
drivers/media/common/saa7146/saa7146_vbi.c | 9 +
include/media/drv-intf/saa7146_vv.h | 1 +
3 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/media/common/saa7146/saa7146_fops.c
b/drivers/media/common/saa7146/saa7146_fops.c
index 930d2c94d5d3..c4664f0da874 100644
--- a/drivers/media/common/saa7146/saa7146_fops.c
+++ b/drivers/media/common/saa7146/saa7146_fops.c
@@ -559,7 +559,7 @@ int saa7146_vv_init(struct saa7146_dev* dev, struct
saa7146_ext_vv *ext_vv)
vbi->start[1] = 312;
vbi->count[1] = 16;
- init_timer(&vv->vbi_read_timeout);
+ timer_setup(&vv->vbi_read_timeout, NULL, 0);
vv->ov_fb.capability = V4L2_FBUF_CAP_LIST_CLIPPING;
vv->ov_fb.flags = V4L2_FBUF_FLAG_PRIMARY;
diff --git a/drivers/media/common/saa7146/saa7146_vbi.c
b/drivers/media/common/saa7146/saa7146_vbi.c
index 69525ca4f52c..ae66c2325228 100644
--- a/drivers/media/common/saa7146/saa7146_vbi.c
+++ b/drivers/media/common/saa7146/saa7146_vbi.c
@@ -348,9 +348,10 @@ static void vbi_stop(struct saa7146_fh *fh, struct file
*file)
spin_unlock_irqrestore(&dev->slock, flags);
}
-static void vbi_read_timeout(unsigned long data)
+static void vbi_read_timeout(struct timer_list *t)
{
- struct file *file = (struct file*)data;
+ struct saa7146_vv *vv = from_timer(vv, t, vbi_read_timeout);
+ struct file *file = vv->vbi_read_timeout_file;
struct saa7146_fh *fh = file->private_data;
struct saa7146_dev *dev = fh->dev;
@@ -401,8 +402,8 @@ static int vbi_open(struct saa7146_dev *dev, struct file
*file)
sizeof(struct saa7146_buf),
file, &dev->v4l2_lock);
- vv->vbi_read_timeout.function = vbi_read_timeout;
- vv->vbi_read_timeout.data = (unsigned long)file;
+ vv->vbi_read_timeout.function = (TIMER_FUNC_TYPE)vbi_read_timeout;
+ vv->vbi_read_timeout_file = file;
/* initialize the brs */
if ( 0 != (SAA7146_USE_PORT_B_FOR_VBI & dev->ext_vv_data->flags)) {
diff --git a/include/media/drv-intf/saa7146_vv.h
b/include/media/drv-intf/saa7146_vv.h
index 736f4f2d8290..926c5b145279 100644
--- a/include/media/drv-intf/saa7146_vv.h
+++ b/include/media/drv-intf/saa7146_vv.h
@@ -107,6 +107,7 @@ struct saa7146_vv
struct saa7146_dmaqueue vbi_dmaq;
struct v4l2_vbi_format vbi_fmt;
struct timer_list vbi_read_timeout;
+ struct file *vbi_read_timeout_file;
/* vbi workaround interrupt queue */
wait_queue_head_t vbi_wq;
int vbi_fieldcount;
--
2.7.4
--
Kees Cook
Pixel Security
[PATCH] media: tvaudio: Convert timers to use timer_setup()
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.
Cc: Mauro Carvalho Chehab
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook
---
drivers/media/i2c/tvaudio.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/media/i2c/tvaudio.c b/drivers/media/i2c/tvaudio.c
index ce86534450ac..16a1e08ce06c 100644
--- a/drivers/media/i2c/tvaudio.c
+++ b/drivers/media/i2c/tvaudio.c
@@ -300,9 +300,9 @@ static int chip_cmd(struct CHIPSTATE *chip, char *name,
audiocmd *cmd)
* if available, ...
*/
-static void chip_thread_wake(unsigned long data)
+static void chip_thread_wake(struct timer_list *t)
{
- struct CHIPSTATE *chip = (struct CHIPSTATE*)data;
+ struct CHIPSTATE *chip = from_timer(chip, t, wt);
wake_up_process(chip->thread);
}
@@ -1995,7 +1995,7 @@ static int tvaudio_probe(struct i2c_client *client, const
struct i2c_device_id *
v4l2_ctrl_handler_setup(&chip->hdl);
chip->thread = NULL;
- init_timer(&chip->wt);
+ timer_setup(&chip->wt, chip_thread_wake, 0);
if (desc->flags & CHIP_NEED_CHECKMODE) {
if (!desc->getrxsubchans || !desc->setaudmode) {
/* This shouldn't be happen. Warn user, but keep working
@@ -2005,8 +2005,6 @@ static int tvaudio_probe(struct i2c_client *client, const
struct i2c_device_id *
return 0;
}
/* start async thread */
- chip->wt.function = chip_thread_wake;
- chip->wt.data = (unsigned long)chip;
chip->thread = kthread_run(chip_thread, chip, "%s",
client->name);
if (IS_ERR(chip->thread)) {
--
2.7.4
--
Kees Cook
Pixel Security
[PATCH] media: saa7146: Convert timers to use timer_setup()
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.
Cc: Hans Verkuil
Cc: Mauro Carvalho Chehab
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook
---
drivers/media/common/saa7146/saa7146_fops.c | 4 ++--
drivers/media/common/saa7146/saa7146_vbi.c | 3 +--
drivers/media/common/saa7146/saa7146_video.c | 3 +--
include/media/drv-intf/saa7146_vv.h | 2 +-
4 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/media/common/saa7146/saa7146_fops.c
b/drivers/media/common/saa7146/saa7146_fops.c
index c4664f0da874..8c87d6837c49 100644
--- a/drivers/media/common/saa7146/saa7146_fops.c
+++ b/drivers/media/common/saa7146/saa7146_fops.c
@@ -163,9 +163,9 @@ void saa7146_buffer_next(struct saa7146_dev *dev,
}
}
-void saa7146_buffer_timeout(unsigned long data)
+void saa7146_buffer_timeout(struct timer_list *t)
{
- struct saa7146_dmaqueue *q = (struct saa7146_dmaqueue*)data;
+ struct saa7146_dmaqueue *q = from_timer(q, t, timeout);
struct saa7146_dev *dev = q->dev;
unsigned long flags;
diff --git a/drivers/media/common/saa7146/saa7146_vbi.c
b/drivers/media/common/saa7146/saa7146_vbi.c
index ae66c2325228..7fa3147c2d7e 100644
--- a/drivers/media/common/saa7146/saa7146_vbi.c
+++ b/drivers/media/common/saa7146/saa7146_vbi.c
@@ -366,8 +366,7 @@ static void vbi_init(struct saa7146_dev *dev, struct
saa7146_vv *vv)
INIT_LIST_HEAD(&vv->vbi_dmaq.queue);
- setup_timer(&vv->vbi_dmaq.timeout, saa7146_buffer_timeout,
- (unsigned long)(&vv->vbi_dmaq));
+ timer_setup(&vv->vbi_dmaq.timeout, saa7146_buffer_timeout, 0);
vv->vbi_dmaq.dev = dev;
init_waitqueue_head(&vv->vbi_wq);
diff --git a/drivers/media/common/saa7146/saa7146_video.c
b/drivers/media/common/saa7146/saa7146_video.c
index 51eeed830de4..2b631eaa65b3 100644
--- a/drivers/media/common/saa7146/saa7146_video.c
+++ b/drivers/media/common/saa7146/saa7146_video.c
@@ -1201,8 +1201,7 @@ static void video_init(struct saa7146_dev *dev, struct
saa7146_vv *vv)
{
INIT_LIST_HEAD(&vv->video_dmaq.queue);
- setup_timer(&vv->video_dmaq.timeout, saa7146_buffer_timeout,
- (unsigned long)(&vv->video_dmaq));
+ timer_setup(&vv->video_dmaq.timeout, saa7146_buffer_timeout, 0);
vv->video_dmaq.dev = dev;
/* set some default values */
diff --git a/include/media/drv-intf/saa7146_vv.h
b/include/media/drv-intf/saa7146_vv.h
index 926c5b145279..17bbe3851d75 100644
--- a/include/media/drv-intf/saa7146_vv.h
+++ b/include/media/drv-intf/saa7146_vv.h
@@ -184,7 +184,7 @@ int saa7146_unregister_device(struct video_device *vid,
struct saa7146_dev *dev)
void saa7146_buffer_finish(struct saa7146_dev *dev, struct saa7146_dmaqueue
*q, int state);
void saa7146_buffer_next(struct saa7146_dev *dev, struct saa7146_dmaqueue
*q,int vbi);
int saa7146_buffer_queue(struct saa7146_dev *dev, struct saa7146_dmaqueue *q,
struct saa7146_buf *buf);
-void saa7146_buffer_timeout(unsigned long data);
+void saa7146_buffer_timeout(struct timer_list *t);
void saa7146_dma_free(struct saa7146_dev* dev,struct videobuf_queue *q,
struct saa7146_buf *buf);
--
2.7.4
--
Kees Cook
Pixel Security
[PATCH] media: dvb-core: Convert timers to use timer_setup()
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.
Cc: Mauro Carvalho Chehab
Cc: devendra sharma
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook
---
drivers/media/dvb-core/dmxdev.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/media/dvb-core/dmxdev.c b/drivers/media/dvb-core/dmxdev.c
index 18e4230865be..3ddd44e1ee77 100644
--- a/drivers/media/dvb-core/dmxdev.c
+++ b/drivers/media/dvb-core/dmxdev.c
@@ -329,9 +329,9 @@ static int dvb_dmxdev_set_buffer_size(struct dmxdev_filter
*dmxdevfilter,
return 0;
}
-static void dvb_dmxdev_filter_timeout(unsigned long data)
+static void dvb_dmxdev_filter_timeout(struct timer_list *t)
{
- struct dmxdev_filter *dmxdevfilter = (struct dmxdev_filter *)data;
+ struct dmxdev_filter *dmxdevfilter = from_timer(dmxdevfilter, t, timer);
dmxdevfilter->buffer.error = -ETIMEDOUT;
spin_lock_irq(&dmxdevfilter->dev->lock);
@@ -346,8 +346,6 @@ static void dvb_dmxdev_filter_timer(struct dmxdev_filter
*dmxdevfilter)
del_timer(&dmxdevfilter->timer);
if (para->timeout) {
- dmxdevfilter->timer.function = dvb_dmxdev_filter_timeout;
- dmxdevfilter->timer.data = (unsigned long)dmxdevfilter;
dmxdevfilter->timer.expires =
jiffies + 1 + (HZ / 2 + HZ * para->timeout) / 1000;
add_timer(&dmxdevfilter->timer);
@@ -754,7 +752,7 @@ static int dvb_demux_open(struct inode *inode, struct file
*file)
dvb_ringbuffer_init(&dmxdevfilter->buffer, NULL, 8192);
dmxdevfilter->type = DMXDEV_TYPE_NONE;
dvb_dmxdev_filter_state_set(dmxdevfilter, DMXDEV_STATE_ALLOCATED);
- init_timer(&dmxdevfilter->timer);
+ timer_setup(&dmxdevfilter->timer, dvb_dmxdev_filter_timeout, 0);
dvbdev->users++;
--
2.7.4
--
Kees Cook
Pixel Security
[PATCH] media: saa7134: Convert timers to use timer_setup()
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.
Cc: Mauro Carvalho Chehab
Cc: Sakari Ailus
Cc: Sean Young
Cc: Geliang Tang
Cc: Hans Verkuil
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook
---
drivers/media/pci/saa7134/saa7134-core.c | 6 +++---
drivers/media/pci/saa7134/saa7134-input.c | 9 -
drivers/media/pci/saa7134/saa7134-ts.c| 3 +--
drivers/media/pci/saa7134/saa7134-vbi.c | 3 +--
drivers/media/pci/saa7134/saa7134-video.c | 3 +--
drivers/media/pci/saa7134/saa7134.h | 2 +-
6 files changed, 11 insertions(+), 15 deletions(-)
diff --git a/drivers/media/pci/saa7134/saa7134-core.c
b/drivers/media/pci/saa7134/saa7134-core.c
index 7976c5a12ca8..9e76de2411ae 100644
--- a/drivers/media/pci/saa7134/saa7134-core.c
+++ b/drivers/media/pci/saa7134/saa7134-core.c
@@ -338,9 +338,9 @@ void saa7134_buffer_next(struct saa7134_dev *dev,
}
}
-void saa7134_buffer_timeout(unsigned long data)
+void saa7134_buffer_timeout(struct timer_list *t)
{
- struct saa7134_dmaqueue *q = (struct saa7134_dmaqueue *)data;
+ struct saa7134_dmaqueue *q = from_timer(q, t, timeout);
struct saa7134_dev *dev = q->dev;
unsigned long flags;
@@ -378,7 +378,7 @@ void saa7134_stop_streaming(struct saa7134_dev *dev, struct
saa7134_dmaqueue *q)
}
}
spin_unlock_irqrestore(&dev->slock, flags);
- saa7134_buffer_timeout((unsigned long)q); /* also calls
del_timer(&q->timeout) */
+ saa7134_buffer_timeout(&q->timeout); /* also calls
del_timer(&q->timeout) */
}
EXPORT_SYMBOL_GPL(saa7134_stop_streaming);
diff --git a/drivers/media/pci/saa7134/saa7134-input.c
b/drivers/media/pci/saa7134/saa7134-input.c
index 9337e4615519..2d5abeddc079 100644
--- a/drivers/media/pci/saa7134/saa7134-input.c
+++ b/drivers/media/pci/saa7134/saa7134-input.c
@@ -447,10 +447,10 @@ void saa7134_input_irq(struct saa7134_dev *dev)
}
}
-static void saa7134_input_timer(unsigned long data)
+static void saa7134_input_timer(struct timer_list *t)
{
- struct saa7134_dev *dev = (struct saa7134_dev *)data;
- struct saa7134_card_ir *ir = dev->remote;
+ struct saa7134_card_ir *ir = from_timer(ir, t, timer);
+ struct saa7134_dev *dev = ir->dev->priv;
build_key(dev);
mod_timer(&ir->timer, jiffies + msecs_to_jiffies(ir->polling));
@@ -507,8 +507,7 @@ static int __saa7134_ir_start(void *priv)
ir->running = true;
if (ir->polling) {
- setup_timer(&ir->timer, saa7134_input_timer,
- (unsigned long)dev);
+ timer_setup(&ir->timer, saa7134_input_timer, 0);
ir->timer.expires = jiffies + HZ;
add_timer(&ir->timer);
}
diff --git a/drivers/media/pci/saa7134/saa7134-ts.c
b/drivers/media/pci/saa7134/saa7134-ts.c
index 7414878af9e0..2be703617e29 100644
--- a/drivers/media/pci/saa7134/saa7134-ts.c
+++ b/drivers/media/pci/saa7134/saa7134-ts.c
@@ -223,8 +223,7 @@ int saa7134_ts_init1(struct saa7134_dev *dev)
dev->ts.nr_packets = ts_nr_packets;
INIT_LIST_HEAD(&dev->ts_q.queue);
- setup_timer(&dev->ts_q.timeout, saa7134_buffer_timeout,
- (unsigned long)(&dev->ts_q));
+ timer_setup(&dev->ts_q.timeout, saa7134_buffer_timeout, 0);
dev->ts_q.dev = dev;
dev->ts_q.need_two = 1;
dev->ts_started= 0;
diff --git a/drivers/media/pci/saa7134/saa7134-vbi.c
b/drivers/media/pci/saa7134/saa7134-vbi.c
index bcad9b2d9bb3..af494c6147f1 100644
--- a/drivers/media/pci/saa7134/saa7134-vbi.c
+++ b/drivers/media/pci/saa7134/saa7134-vbi.c
@@ -181,8 +181,7 @@ struct vb2_ops saa7134_vbi_qops = {
int saa7134_vbi_init1(struct saa7134_dev *dev)
{
INIT_LIST_HEAD(&dev->vbi_q.queue);
- setup_timer(&dev->vbi_q.timeout, saa7134_buffer_timeout,
- (unsigned long)(&dev->vbi_q));
+ timer_setup(&dev->vbi_q.timeout, saa7134_buffer_timeout, 0);
dev->vbi_q.dev = dev;
if (vbibufs < 2)
diff --git a/drivers/media/pci/saa7134/saa7134-video.c
b/drivers/media/pci/saa7134/saa7134-video.c
index 51d42bbf969e..82d2a24644e4 100644
--- a/drivers/media/pci/saa7134/saa7134-video.c
+++ b/drivers/media/pci/saa7134/saa7134-video.c
@@ -2145,8 +2145,7 @@ int saa7134_video_init1(struct saa7134_dev *dev)
dev->automute = 0;
INIT_LIST_HEAD(&dev->video_q.queue);
- setup_timer(&dev->video_q.timeout, saa7134_buffer_timeout,
- (unsigned long)(&dev->video_q));
+ timer_setup(&dev->video_q.timeout, saa7134_buffer_timeout, 0);
dev->
[PATCH] media: tc358743: Convert timers to use timer_setup()
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.
Cc: Mats Randgaard
Cc: Mauro Carvalho Chehab
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook
---
drivers/media/i2c/tc358743.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index a9355032076f..359f63d7dfca 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -1481,9 +1481,9 @@ static irqreturn_t tc358743_irq_handler(int irq, void
*dev_id)
return handled ? IRQ_HANDLED : IRQ_NONE;
}
-static void tc358743_irq_poll_timer(unsigned long arg)
+static void tc358743_irq_poll_timer(struct timer_list *t)
{
- struct tc358743_state *state = (struct tc358743_state *)arg;
+ struct tc358743_state *state = from_timer(state, t, timer);
unsigned int msecs;
schedule_work(&state->work_i2c_poll);
@@ -2147,8 +2147,7 @@ static int tc358743_probe(struct i2c_client *client,
} else {
INIT_WORK(&state->work_i2c_poll,
tc358743_work_i2c_poll);
- setup_timer(&state->timer, tc358743_irq_poll_timer,
- (unsigned long)state);
+ timer_setup(&state->timer, tc358743_irq_poll_timer, 0);
state->timer.expires = jiffies +
msecs_to_jiffies(POLL_INTERVAL_MS);
add_timer(&state->timer);
--
2.7.4
--
Kees Cook
Pixel Security
[PATCH] media: serial_ir: Convert timers to use timer_setup()
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.
Cc: Sean Young
Cc: Mauro Carvalho Chehab
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook
---
drivers/media/rc/serial_ir.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/media/rc/serial_ir.c b/drivers/media/rc/serial_ir.c
index 8b66926bc16a..8bf5637b3a69 100644
--- a/drivers/media/rc/serial_ir.c
+++ b/drivers/media/rc/serial_ir.c
@@ -470,7 +470,7 @@ static int hardware_init_port(void)
return 0;
}
-static void serial_ir_timeout(unsigned long arg)
+static void serial_ir_timeout(struct timer_list *unused)
{
DEFINE_IR_RAW_EVENT(ev);
@@ -540,8 +540,7 @@ static int serial_ir_probe(struct platform_device *dev)
serial_ir.rcdev = rcdev;
- setup_timer(&serial_ir.timeout_timer, serial_ir_timeout,
- (unsigned long)&serial_ir);
+ timer_setup(&serial_ir.timeout_timer, serial_ir_timeout, 0);
result = devm_request_irq(&dev->dev, irq, serial_ir_irq_handler,
share_irq ? IRQF_SHARED : 0,
--
2.7.4
--
Kees Cook
Pixel Security
[PATCH] media: input: Convert timers to use timer_setup()
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.
One input_dev user hijacks the input_dev software autorepeat timer to
perform its own repeat management. However, there is no path back to the
existing status variable, so add a generic one to the input structure and
use that instead.
Cc: Dmitry Torokhov
Cc: Mauro Carvalho Chehab
Cc: Hans Verkuil
Cc: Sakari Ailus
Cc: Geliang Tang
Cc: linux-in...@vger.kernel.org
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook
Acked-by: Pali Rohár
---
drivers/input/input.c | 12 ++--
drivers/media/pci/ttpci/av7110.h| 1 -
drivers/media/pci/ttpci/av7110_ir.c | 16
include/linux/input.h | 2 ++
4 files changed, 16 insertions(+), 15 deletions(-)
diff --git a/drivers/input/input.c b/drivers/input/input.c
index d268fdc23c64..497ad2dcb699 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -76,7 +76,7 @@ static void input_start_autorepeat(struct input_dev *dev, int
code)
{
if (test_bit(EV_REP, dev->evbit) &&
dev->rep[REP_PERIOD] && dev->rep[REP_DELAY] &&
- dev->timer.data) {
+ dev->timer.function) {
dev->repeat_key = code;
mod_timer(&dev->timer,
jiffies + msecs_to_jiffies(dev->rep[REP_DELAY]));
@@ -179,9 +179,9 @@ static void input_pass_event(struct input_dev *dev,
* dev->event_lock here to avoid racing with input_event
* which may cause keys get "stuck".
*/
-static void input_repeat_key(unsigned long data)
+static void input_repeat_key(struct timer_list *t)
{
- struct input_dev *dev = (void *) data;
+ struct input_dev *dev = from_timer(dev, t, timer);
unsigned long flags;
spin_lock_irqsave(&dev->event_lock, flags);
@@ -1790,7 +1790,7 @@ struct input_dev *input_allocate_device(void)
device_initialize(&dev->dev);
mutex_init(&dev->mutex);
spin_lock_init(&dev->event_lock);
- init_timer(&dev->timer);
+ timer_setup(&dev->timer, NULL, 0);
INIT_LIST_HEAD(&dev->h_list);
INIT_LIST_HEAD(&dev->node);
@@ -2053,8 +2053,8 @@ static void devm_input_device_unregister(struct device
*dev, void *res)
*/
void input_enable_softrepeat(struct input_dev *dev, int delay, int period)
{
- dev->timer.data = (unsigned long) dev;
- dev->timer.function = input_repeat_key;
+ dev->timer.function = (TIMER_FUNC_TYPE)input_repeat_key;
+ dev->timer_data = 0;
dev->rep[REP_DELAY] = delay;
dev->rep[REP_PERIOD] = period;
}
diff --git a/drivers/media/pci/ttpci/av7110.h b/drivers/media/pci/ttpci/av7110.h
index 347827925c14..b98a4f3006df 100644
--- a/drivers/media/pci/ttpci/av7110.h
+++ b/drivers/media/pci/ttpci/av7110.h
@@ -93,7 +93,6 @@ struct infrared {
u8 inversion;
u16 last_key;
u16 last_toggle;
- u8 delay_timer_finished;
};
diff --git a/drivers/media/pci/ttpci/av7110_ir.c
b/drivers/media/pci/ttpci/av7110_ir.c
index ca05198de2c2..a883caa6488c 100644
--- a/drivers/media/pci/ttpci/av7110_ir.c
+++ b/drivers/media/pci/ttpci/av7110_ir.c
@@ -155,16 +155,16 @@ static void av7110_emit_key(unsigned long parm)
if (timer_pending(&ir->keyup_timer)) {
del_timer(&ir->keyup_timer);
if (ir->last_key != keycode || toggle != ir->last_toggle) {
- ir->delay_timer_finished = 0;
+ ir->input_dev->timer_data = 0;
input_event(ir->input_dev, EV_KEY, ir->last_key, 0);
input_event(ir->input_dev, EV_KEY, keycode, 1);
input_sync(ir->input_dev);
- } else if (ir->delay_timer_finished) {
+ } else if (ir->input_dev->timer_data) {
input_event(ir->input_dev, EV_KEY, keycode, 2);
input_sync(ir->input_dev);
}
} else {
- ir->delay_timer_finished = 0;
+ ir->input_dev->timer_data = 0;
input_event(ir->input_dev, EV_KEY, keycode, 1);
input_sync(ir->input_dev);
}
@@ -206,11 +206,12 @@ static void input_register_keys(struct infrared *ir)
/* called by the input driver after rep[REP_DELAY] ms */
-static void input_repeat_key(unsigned long parm)
+static void input_repeat_key(struct timer_list *t)
{
- struct infrared *ir = (struct infrared *) parm;
+ struct inp
[PATCH] staging/atomisp: Convert timers to use timer_setup()
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.
Cc: Mauro Carvalho Chehab
Cc: Greg Kroah-Hartman
Cc: Alan Cox
Cc: Daeseok Youn
Cc: Arnd Bergmann
Cc: linux-media@vger.kernel.org
Cc: de...@driverdev.osuosl.org
Signed-off-by: Kees Cook
---
drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c | 13 -
drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.h | 6 +-
.../media/atomisp/pci/atomisp2/atomisp_compat_css20.c | 2 +-
drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c | 15 +--
4 files changed, 11 insertions(+), 25 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
index b0c647f4d250..f3cf4ecba630 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
@@ -1661,20 +1661,15 @@ void atomisp_css_flush(struct atomisp_device *isp)
dev_dbg(isp->dev, "atomisp css flush done\n");
}
-#ifndef ISP2401
-void atomisp_wdt(unsigned long isp_addr)
-#else
-void atomisp_wdt(unsigned long pipe_addr)
-#endif
+void atomisp_wdt(struct timer_list *t)
{
#ifndef ISP2401
- struct atomisp_device *isp = (struct atomisp_device *)isp_addr;
+ struct atomisp_sub_device *asd = from_timer(asd, t, wdt);
#else
- struct atomisp_video_pipe *pipe =
- (struct atomisp_video_pipe *)pipe_addr;
+ struct atomisp_video_pipe *pipe = from_timer(pipe, t, wdt);
struct atomisp_sub_device *asd = pipe->asd;
- struct atomisp_device *isp = asd->isp;
#endif
+ struct atomisp_device *isp = asd->isp;
#ifdef ISP2401
atomic_inc(&pipe->wdt_count);
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.h
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.h
index 31ba4e613d13..4bb83864da2e 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.h
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.h
@@ -85,11 +85,7 @@ static inline void __iomem
*atomisp_get_io_virt_addr(unsigned int address)
void atomisp_msi_irq_init(struct atomisp_device *isp, struct pci_dev *dev);
void atomisp_msi_irq_uninit(struct atomisp_device *isp, struct pci_dev *dev);
void atomisp_wdt_work(struct work_struct *work);
-#ifndef ISP2401
-void atomisp_wdt(unsigned long isp_addr);
-#else
-void atomisp_wdt(unsigned long pipe_addr);
-#endif
+void atomisp_wdt(struct timer_list *t);
void atomisp_setup_flash(struct atomisp_sub_device *asd);
irqreturn_t atomisp_isr(int irq, void *dev);
irqreturn_t atomisp_isr_thread(int irq, void *isp_ptr);
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c
index 5027fd20d966..b190f9958066 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c
@@ -4526,7 +4526,7 @@ int atomisp_css_isr_thread(struct atomisp_device *isp,
for (i = 0; i < isp->num_of_streams; i++)
atomisp_wdt_stop(&isp->asd[i], 0);
#ifndef ISP2401
- atomisp_wdt((unsigned long)isp);
+ atomisp_wdt(&isp->asd[0].wdt);
#else
queue_work(isp->wdt_work_queue, &isp->wdt_work);
#endif
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c
index e85b3819bffa..20aff7faf44a 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_v4l2.c
@@ -1150,17 +1150,12 @@ static int init_atomisp_wdts(struct atomisp_device *isp)
struct atomisp_sub_device *asd = &isp->asd[i];
asd = &isp->asd[i];
#ifndef ISP2401
- setup_timer(&asd->wdt, atomisp_wdt, (unsigned long)isp);
+ timer_setup(&asd->wdt, atomisp_wdt, 0);
#else
- setup_timer(&asd->video_out_capture.wdt,
- atomisp_wdt, (unsigned long)&asd->video_out_capture);
- setup_timer(&asd->video_out_preview.wdt,
- atomisp_wdt, (unsigned long)&asd->video_out_preview);
- setup_timer(&asd->video_out_vf.wdt,
- atomisp_wdt, (unsigned long)&asd->video_out_vf);
- setup_timer(&asd->video_out_video_capture.wdt,
- atomisp_wdt,
- (unsigned long)&asd->video_out_video_capture);
+ timer_setup(&asd->video_out_capture.wdt, atomisp_wdt, 0);
+ timer_set
Re: [PATCH] staging/atomisp: Convert timers to use timer_setup()
On Tue, Oct 17, 2017 at 1:23 AM, Sakari Ailus wrote: > On Mon, Oct 16, 2017 at 04:24:56PM -0700, Kees Cook wrote: >> In preparation for unconditionally passing the struct timer_list pointer to >> all timer callbacks, switch to using the new timer_setup() and from_timer() >> to pass the timer pointer explicitly. >> >> Cc: Mauro Carvalho Chehab >> Cc: Greg Kroah-Hartman >> Cc: Alan Cox >> Cc: Daeseok Youn >> Cc: Arnd Bergmann >> Cc: linux-media@vger.kernel.org >> Cc: de...@driverdev.osuosl.org >> Signed-off-by: Kees Cook > > This appears to be the same as the patch I've applied previously. Okay, sorry for the noise. I didn't see it in -next when I did my rebase this week. -Kees -- Kees Cook Pixel Security
Re: [PATCH] media: input: Convert timers to use timer_setup()
On Thu, Oct 19, 2017 at 3:32 PM, Dmitry Torokhov wrote: > On Mon, Oct 16, 2017 at 04:14:43PM -0700, Kees Cook wrote: >> In preparation for unconditionally passing the struct timer_list pointer to >> all timer callbacks, switch to using the new timer_setup() and from_timer() >> to pass the timer pointer explicitly. >> >> One input_dev user hijacks the input_dev software autorepeat timer to >> perform its own repeat management. However, there is no path back to the >> existing status variable, so add a generic one to the input structure and >> use that instead. > > That is too bad and it should not be doing this. I'd rather av7110 used > its own private timer for that. Yeah, that was a pretty weird case. I couldn't see how to avoid it, though. I didn't see a way to hook the autorepeat, but I'm not too familiar with the code here. (I am finding such bizarre stuff with this refactoring...) -Kees -- Kees Cook Pixel Security
[PATCH] staging: atomisp: i2c: Convert timers to use timer_setup()
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.
Signed-off-by: Kees Cook
---
drivers/staging/media/atomisp/i2c/lm3554.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/media/atomisp/i2c/lm3554.c
b/drivers/staging/media/atomisp/i2c/lm3554.c
index 679176f7c542..a815f208409f 100644
--- a/drivers/staging/media/atomisp/i2c/lm3554.c
+++ b/drivers/staging/media/atomisp/i2c/lm3554.c
@@ -171,10 +171,9 @@ static int lm3554_set_config1(struct lm3554 *flash)
/*
-
* Hardware trigger
*/
-static void lm3554_flash_off_delay(long unsigned int arg)
+static void lm3554_flash_off_delay(struct timer_list *t)
{
- struct v4l2_subdev *sd = i2c_get_clientdata((struct i2c_client *)arg);
- struct lm3554 *flash = to_lm3554(sd);
+ struct lm3554 *flash = from_timer(flash, t, flash_off_delay);
struct lm3554_platform_data *pdata = flash->pdata;
gpio_set_value(pdata->gpio_strobe, 0);
@@ -915,8 +914,7 @@ static int lm3554_probe(struct i2c_client *client,
mutex_init(&flash->power_lock);
- setup_timer(&flash->flash_off_delay, lm3554_flash_off_delay,
- (unsigned long)client);
+ timer_setup(&flash->flash_off_delay, lm3554_flash_off_delay, 0);
err = lm3554_gpio_init(client);
if (err) {
--
2.7.4
--
Kees Cook
Pixel Security
Re: [PATCH] media: input: Convert timers to use timer_setup()
On Thu, Oct 19, 2017 at 3:48 PM, Dmitry Torokhov
wrote:
> On Thu, Oct 19, 2017 at 03:45:38PM -0700, Kees Cook wrote:
>> On Thu, Oct 19, 2017 at 3:32 PM, Dmitry Torokhov
>> wrote:
>> > On Mon, Oct 16, 2017 at 04:14:43PM -0700, Kees Cook wrote:
>> >> In preparation for unconditionally passing the struct timer_list pointer
>> >> to
>> >> all timer callbacks, switch to using the new timer_setup() and
>> >> from_timer()
>> >> to pass the timer pointer explicitly.
>> >>
>> >> One input_dev user hijacks the input_dev software autorepeat timer to
>> >> perform its own repeat management. However, there is no path back to the
>> >> existing status variable, so add a generic one to the input structure and
>> >> use that instead.
>> >
>> > That is too bad and it should not be doing this. I'd rather av7110 used
>> > its own private timer for that.
>>
>> Yeah, that was a pretty weird case. I couldn't see how to avoid it,
>> though. I didn't see a way to hook the autorepeat, but I'm not too
>> familiar with the code here.
>
> You just need to manage the private timer in the driver and not mess up
> with the input core if input core's autorepeat does not provide the
> desired behavior...
I don't know how to fix this, but I still need to do this refactoring.
What's the correct step forward here? Should I temporarily disable the
timer in av7110?
Seems like the hijacking was introduced in ee820a648fb3 ("V4L/DVB
(5334): Dvb-ttpci: Infrared remote control refactoring").
Thanks!
-Kees
--
Kees Cook
Pixel Security
[PATCH] media: pci: Convert timers to use timer_setup()
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.
Cc: Mauro Carvalho Chehab
Cc: Andy Walls
Cc: Sergey Kozlov
Cc: Abylay Ospan
Cc: Ezequiel Garcia
Cc: Hans Verkuil
Cc: Arvind Yadav
Cc: Geliang Tang
Cc: Sean Young
Cc: Sakari Ailus
Cc: "Pali Rohár"
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook
---
drivers/media/pci/bt8xx/bttv-driver.c | 6 +++---
drivers/media/pci/bt8xx/bttv-input.c | 19 ++-
drivers/media/pci/bt8xx/bttvp.h| 1 +
drivers/media/pci/cx18/cx18-fileops.c | 4 ++--
drivers/media/pci/cx18/cx18-fileops.h | 2 +-
drivers/media/pci/cx18/cx18-streams.c | 2 +-
drivers/media/pci/ivtv/ivtv-driver.c | 3 +--
drivers/media/pci/ivtv/ivtv-irq.c | 4 ++--
drivers/media/pci/ivtv/ivtv-irq.h | 2 +-
drivers/media/pci/netup_unidvb/netup_unidvb_core.c | 7 +++
drivers/media/pci/ttpci/av7110_ir.c| 7 +++
drivers/media/pci/tw686x/tw686x-core.c | 7 +++
12 files changed, 31 insertions(+), 33 deletions(-)
diff --git a/drivers/media/pci/bt8xx/bttv-driver.c
b/drivers/media/pci/bt8xx/bttv-driver.c
index 227086a2e99c..b366a7e1d976 100644
--- a/drivers/media/pci/bt8xx/bttv-driver.c
+++ b/drivers/media/pci/bt8xx/bttv-driver.c
@@ -3652,9 +3652,9 @@ bttv_irq_wakeup_vbi(struct bttv *btv, struct bttv_buffer
*wakeup,
wake_up(&wakeup->vb.done);
}
-static void bttv_irq_timeout(unsigned long data)
+static void bttv_irq_timeout(struct timer_list *t)
{
- struct bttv *btv = (struct bttv *)data;
+ struct bttv *btv = from_timer(btv, t, timeout);
struct bttv_buffer_set old,new;
struct bttv_buffer *ovbi;
struct bttv_buffer *item;
@@ -4043,7 +4043,7 @@ static int bttv_probe(struct pci_dev *dev, const struct
pci_device_id *pci_id)
INIT_LIST_HEAD(&btv->capture);
INIT_LIST_HEAD(&btv->vcapture);
- setup_timer(&btv->timeout, bttv_irq_timeout, (unsigned long)btv);
+ timer_setup(&btv->timeout, bttv_irq_timeout, 0);
btv->i2c_rc = -1;
btv->tuner_type = UNSET;
diff --git a/drivers/media/pci/bt8xx/bttv-input.c
b/drivers/media/pci/bt8xx/bttv-input.c
index 73d655d073d6..ac7674700685 100644
--- a/drivers/media/pci/bt8xx/bttv-input.c
+++ b/drivers/media/pci/bt8xx/bttv-input.c
@@ -133,10 +133,10 @@ void bttv_input_irq(struct bttv *btv)
ir_handle_key(btv);
}
-static void bttv_input_timer(unsigned long data)
+static void bttv_input_timer(struct timer_list *t)
{
- struct bttv *btv = (struct bttv*)data;
- struct bttv_ir *ir = btv->remote;
+ struct bttv_ir *ir = from_timer(ir, t, timer);
+ struct bttv *btv = ir->btv;
if (btv->c.type == BTTV_BOARD_ENLTV_FM_2)
ir_enltv_handle_key(btv);
@@ -189,9 +189,9 @@ static u32 bttv_rc5_decode(unsigned int code)
return rc5;
}
-static void bttv_rc5_timer_end(unsigned long data)
+static void bttv_rc5_timer_end(struct timer_list *t)
{
- struct bttv_ir *ir = (struct bttv_ir *)data;
+ struct bttv_ir *ir = from_timer(ir, t, timer);
ktime_t tv;
u32 gap, rc5, scancode;
u8 toggle, command, system;
@@ -296,15 +296,15 @@ static int bttv_rc5_irq(struct bttv *btv)
/* -- */
-static void bttv_ir_start(struct bttv *btv, struct bttv_ir *ir)
+static void bttv_ir_start(struct bttv_ir *ir)
{
if (ir->polling) {
- setup_timer(&ir->timer, bttv_input_timer, (unsigned long)btv);
+ timer_setup(&ir->timer, bttv_input_timer, 0);
ir->timer.expires = jiffies + msecs_to_jiffies(1000);
add_timer(&ir->timer);
} else if (ir->rc5_gpio) {
/* set timer_end for code completion */
- setup_timer(&ir->timer, bttv_rc5_timer_end, (unsigned long)ir);
+ timer_setup(&ir->timer, bttv_rc5_timer_end, 0);
ir->shift_by = 1;
ir->rc5_remote_gap = ir_rc5_remote_gap;
}
@@ -531,6 +531,7 @@ int bttv_input_init(struct bttv *btv)
/* init input device */
ir->dev = rc;
+ ir->btv = btv;
snprintf(ir->name, sizeof(ir->name), "bttv IR (card=%d)",
btv->c.type);
@@ -553,7 +554,7 @@ int bttv_input_init(struct bttv *btv)
rc->driver_name = MODULE_NAME;
btv->remote = ir;
- bttv_ir_start(btv, ir);
+ bttv_ir_start(ir);
/* all done */
err = rc_register_device(rc);
diff --git a/drivers/media/pci/bt8xx/bttvp.h b/drivers/media/pci/bt8xx/bttvp.h
index 9efc455
[PATCH] media: pvrusb2: Convert timers to use timer_setup()
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.
Cc: Mike Isely
Cc: Mauro Carvalho Chehab
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook
---
drivers/media/usb/pvrusb2/pvrusb2-hdw.c | 64 ++---
1 file changed, 36 insertions(+), 28 deletions(-)
diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
index ad5b25b89699..8289ee482f49 100644
--- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
+++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
@@ -330,10 +330,10 @@ static void pvr2_hdw_state_log_state(struct pvr2_hdw *);
static int pvr2_hdw_cmd_usbstream(struct pvr2_hdw *hdw,int runFl);
static int pvr2_hdw_commit_setup(struct pvr2_hdw *hdw);
static int pvr2_hdw_get_eeprom_addr(struct pvr2_hdw *hdw);
-static void pvr2_hdw_quiescent_timeout(unsigned long);
-static void pvr2_hdw_decoder_stabilization_timeout(unsigned long);
-static void pvr2_hdw_encoder_wait_timeout(unsigned long);
-static void pvr2_hdw_encoder_run_timeout(unsigned long);
+static void pvr2_hdw_quiescent_timeout(struct timer_list *);
+static void pvr2_hdw_decoder_stabilization_timeout(struct timer_list *);
+static void pvr2_hdw_encoder_wait_timeout(struct timer_list *);
+static void pvr2_hdw_encoder_run_timeout(struct timer_list *);
static int pvr2_issue_simple_cmd(struct pvr2_hdw *,u32);
static int pvr2_send_request_ex(struct pvr2_hdw *hdw,
unsigned int timeout,int probe_fl,
@@ -2373,18 +2373,15 @@ struct pvr2_hdw *pvr2_hdw_create(struct usb_interface
*intf,
}
if (!hdw) goto fail;
- setup_timer(&hdw->quiescent_timer, pvr2_hdw_quiescent_timeout,
- (unsigned long)hdw);
+ timer_setup(&hdw->quiescent_timer, pvr2_hdw_quiescent_timeout, 0);
- setup_timer(&hdw->decoder_stabilization_timer,
- pvr2_hdw_decoder_stabilization_timeout,
- (unsigned long)hdw);
+ timer_setup(&hdw->decoder_stabilization_timer,
+ pvr2_hdw_decoder_stabilization_timeout, 0);
- setup_timer(&hdw->encoder_wait_timer, pvr2_hdw_encoder_wait_timeout,
- (unsigned long)hdw);
+ timer_setup(&hdw->encoder_wait_timer, pvr2_hdw_encoder_wait_timeout,
+ 0);
- setup_timer(&hdw->encoder_run_timer, pvr2_hdw_encoder_run_timeout,
- (unsigned long)hdw);
+ timer_setup(&hdw->encoder_run_timer, pvr2_hdw_encoder_run_timeout, 0);
hdw->master_state = PVR2_STATE_DEAD;
@@ -3539,10 +3536,16 @@ static void pvr2_ctl_read_complete(struct urb *urb)
complete(&hdw->ctl_done);
}
+struct hdw_timer {
+ struct timer_list timer;
+ struct pvr2_hdw *hdw;
+};
-static void pvr2_ctl_timeout(unsigned long data)
+static void pvr2_ctl_timeout(struct timer_list *t)
{
- struct pvr2_hdw *hdw = (struct pvr2_hdw *)data;
+ struct hdw_timer *timer = from_timer(timer, t, timer);
+ struct pvr2_hdw *hdw = timer->hdw;
+
if (hdw->ctl_write_pend_flag || hdw->ctl_read_pend_flag) {
hdw->ctl_timeout_flag = !0;
if (hdw->ctl_write_pend_flag)
@@ -3564,7 +3567,10 @@ static int pvr2_send_request_ex(struct pvr2_hdw *hdw,
{
unsigned int idx;
int status = 0;
- struct timer_list timer;
+ struct hdw_timer timer = {
+ .hdw = hdw,
+ };
+
if (!hdw->ctl_lock_held) {
pvr2_trace(PVR2_TRACE_ERROR_LEGS,
"Attempted to execute control transfer without
lock!!");
@@ -3621,8 +3627,8 @@ static int pvr2_send_request_ex(struct pvr2_hdw *hdw,
hdw->ctl_timeout_flag = 0;
hdw->ctl_write_pend_flag = 0;
hdw->ctl_read_pend_flag = 0;
- setup_timer(&timer, pvr2_ctl_timeout, (unsigned long)hdw);
- timer.expires = jiffies + timeout;
+ timer_setup_on_stack(&timer.timer, pvr2_ctl_timeout, 0);
+ timer.timer.expires = jiffies + timeout;
if (write_len && write_data) {
hdw->cmd_debug_state = 2;
@@ -3677,7 +3683,7 @@ status);
}
/* Start timer */
- add_timer(&timer);
+ add_timer(&timer.timer);
/* Now wait for all I/O to complete */
hdw->cmd_debug_state = 4;
@@ -3687,7 +3693,7 @@ status);
hdw->cmd_debug_state = 5;
/* Stop timer */
- del_timer_sync(&timer);
+ del_timer_sync(&timer.timer);
hdw->cmd_debug_state = 6;
status = 0;
@@ -3769,6 +3775,8 @@ status);
if ((status < 0) && (!probe_fl)) {
pvr2_hdw_render_useless(hdw);
}
+ destroy_timer_on_stack(&timer.timer);
+
return status;
}
@@ -
[PATCH] media: rc: Convert timers to use timer_setup()
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.
Cc: Maxim Levitsky
Cc: Mauro Carvalho Chehab
Cc: Sean Young
Cc: James Hogan
Cc: Hans Verkuil
Cc: "Antti Seppälä"
Cc: Heiner Kallweit
Cc: "David Härdeman"
Cc: Andi Shyti
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook
---
drivers/media/rc/ene_ir.c | 7 +++
drivers/media/rc/igorplugusb.c| 6 +++---
drivers/media/rc/img-ir/img-ir-hw.c | 13 ++---
drivers/media/rc/img-ir/img-ir-raw.c | 6 +++---
drivers/media/rc/imon.c | 7 +++
drivers/media/rc/ir-mce_kbd-decoder.c | 7 +++
drivers/media/rc/rc-ir-raw.c | 8
drivers/media/rc/rc-main.c| 7 +++
drivers/media/rc/sir_ir.c | 4 ++--
9 files changed, 30 insertions(+), 35 deletions(-)
diff --git a/drivers/media/rc/ene_ir.c b/drivers/media/rc/ene_ir.c
index af7ba23e16e1..71b8c9bbf6c4 100644
--- a/drivers/media/rc/ene_ir.c
+++ b/drivers/media/rc/ene_ir.c
@@ -670,9 +670,9 @@ static void ene_tx_sample(struct ene_device *dev)
}
/* timer to simulate tx done interrupt */
-static void ene_tx_irqsim(unsigned long data)
+static void ene_tx_irqsim(struct timer_list *t)
{
- struct ene_device *dev = (struct ene_device *)data;
+ struct ene_device *dev = from_timer(dev, t, tx_sim_timer);
unsigned long flags;
spin_lock_irqsave(&dev->hw_lock, flags);
@@ -1045,8 +1045,7 @@ static int ene_probe(struct pnp_dev *pnp_dev, const
struct pnp_device_id *id)
if (!dev->hw_learning_and_tx_capable && txsim) {
dev->hw_learning_and_tx_capable = true;
- setup_timer(&dev->tx_sim_timer, ene_tx_irqsim,
- (long unsigned int)dev);
+ timer_setup(&dev->tx_sim_timer, ene_tx_irqsim, 0);
pr_warn("Simulation of TX activated\n");
}
diff --git a/drivers/media/rc/igorplugusb.c b/drivers/media/rc/igorplugusb.c
index 4b715eb995f8..f563ddd7f739 100644
--- a/drivers/media/rc/igorplugusb.c
+++ b/drivers/media/rc/igorplugusb.c
@@ -137,9 +137,9 @@ static void igorplugusb_cmd(struct igorplugusb *ir, int cmd)
dev_err(ir->dev, "submit urb failed: %d", ret);
}
-static void igorplugusb_timer(unsigned long data)
+static void igorplugusb_timer(struct timer_list *t)
{
- struct igorplugusb *ir = (struct igorplugusb *)data;
+ struct igorplugusb *ir = from_timer(ir, t, timer);
igorplugusb_cmd(ir, GET_INFRACODE);
}
@@ -174,7 +174,7 @@ static int igorplugusb_probe(struct usb_interface *intf,
ir->dev = &intf->dev;
- setup_timer(&ir->timer, igorplugusb_timer, (unsigned long)ir);
+ timer_setup(&ir->timer, igorplugusb_timer, 0);
ir->request.bRequest = GET_INFRACODE;
ir->request.bRequestType = USB_TYPE_VENDOR | USB_DIR_IN;
diff --git a/drivers/media/rc/img-ir/img-ir-hw.c
b/drivers/media/rc/img-ir/img-ir-hw.c
index 82fdf4cc0824..f54bc5d23893 100644
--- a/drivers/media/rc/img-ir/img-ir-hw.c
+++ b/drivers/media/rc/img-ir/img-ir-hw.c
@@ -867,9 +867,9 @@ static void img_ir_handle_data(struct img_ir_priv *priv,
u32 len, u64 raw)
}
/* timer function to end waiting for repeat. */
-static void img_ir_end_timer(unsigned long arg)
+static void img_ir_end_timer(struct timer_list *t)
{
- struct img_ir_priv *priv = (struct img_ir_priv *)arg;
+ struct img_ir_priv *priv = from_timer(priv, t, hw.end_timer);
spin_lock_irq(&priv->lock);
img_ir_end_repeat(priv);
@@ -881,9 +881,9 @@ static void img_ir_end_timer(unsigned long arg)
* cleared when invalid interrupts were generated due to a quirk in the
* img-ir decoder.
*/
-static void img_ir_suspend_timer(unsigned long arg)
+static void img_ir_suspend_timer(struct timer_list *t)
{
- struct img_ir_priv *priv = (struct img_ir_priv *)arg;
+ struct img_ir_priv *priv = from_timer(priv, t, hw.suspend_timer);
spin_lock_irq(&priv->lock);
/*
@@ -1055,9 +1055,8 @@ int img_ir_probe_hw(struct img_ir_priv *priv)
img_ir_probe_hw_caps(priv);
/* Set up the end timer */
- setup_timer(&hw->end_timer, img_ir_end_timer, (unsigned long)priv);
- setup_timer(&hw->suspend_timer, img_ir_suspend_timer,
- (unsigned long)priv);
+ timer_setup(&hw->end_timer, img_ir_end_timer, 0);
+ timer_setup(&hw->suspend_timer, img_ir_suspend_timer, 0);
/* Register a clock notifier */
if (!IS_ERR(priv->clk)) {
diff --git a/drivers/media/rc/img-ir/img-ir-raw.c
b/drivers/media/rc/img-ir/img-ir-raw.c
index 64714efc1145..6e545680d3b6 100644
--- a/drivers/media/rc/img-ir/img-ir-raw.c
[PATCH] media: s2255: Convert timers to use timer_setup()
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.
Cc: Mauro Carvalho Chehab
Cc: Laurent Pinchart
Cc: Sakari Ailus
Cc: Bhumika Goyal
Cc: Mike Isely
Cc: Arvind Yadav
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook
---
drivers/media/usb/s2255/s2255drv.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/media/usb/s2255/s2255drv.c
b/drivers/media/usb/s2255/s2255drv.c
index b2f239c4ba42..7fee5766587a 100644
--- a/drivers/media/usb/s2255/s2255drv.c
+++ b/drivers/media/usb/s2255/s2255drv.c
@@ -485,9 +485,10 @@ static void s2255_reset_dsppower(struct s2255_dev *dev)
/* kickstarts the firmware loading. from probe
*/
-static void s2255_timer(unsigned long user_data)
+static void s2255_timer(struct timer_list *t)
{
- struct s2255_fw *data = (struct s2255_fw *)user_data;
+ struct s2255_dev *dev = from_timer(dev, t, timer);
+ struct s2255_fw *data = dev->fw_data;
if (usb_submit_urb(data->fw_urb, GFP_ATOMIC) < 0) {
pr_err("s2255: can't submit urb\n");
atomic_set(&data->fw_state, S2255_FW_FAILED);
@@ -2283,7 +2284,7 @@ static int s2255_probe(struct usb_interface *interface,
dev_err(&interface->dev, "Could not find bulk-in endpoint\n");
goto errorEP;
}
- setup_timer(&dev->timer, s2255_timer, (unsigned long)dev->fw_data);
+ timer_setup(&dev->timer, s2255_timer, 0);
init_waitqueue_head(&dev->fw_data->wait_fw);
for (i = 0; i < MAX_CHANNELS; i++) {
struct s2255_vc *vc = &dev->vc[i];
--
2.7.4
--
Kees Cook
Pixel Security
[PATCH] media: radio: Convert timers to use timer_setup()
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.
Cc: Hans Verkuil
Cc: Mauro Carvalho Chehab
Cc: "David S. Miller"
Cc: Johannes Berg
Cc: linux-media@vger.kernel.org
Signed-off-by: Kees Cook
---
drivers/media/radio/radio-cadet.c | 7 +++
drivers/media/radio/wl128x/fmdrv_common.c | 7 +++
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/media/radio/radio-cadet.c
b/drivers/media/radio/radio-cadet.c
index 6888b7db449d..7575e5370a49 100644
--- a/drivers/media/radio/radio-cadet.c
+++ b/drivers/media/radio/radio-cadet.c
@@ -281,9 +281,9 @@ static bool cadet_has_rds_data(struct cadet *dev)
}
-static void cadet_handler(unsigned long data)
+static void cadet_handler(struct timer_list *t)
{
- struct cadet *dev = (void *)data;
+ struct cadet *dev = from_timer(dev, t, readtimer);
/* Service the RDS fifo */
if (mutex_trylock(&dev->lock)) {
@@ -309,7 +309,6 @@ static void cadet_handler(unsigned long data)
/*
* Clean up and exit
*/
- setup_timer(&dev->readtimer, cadet_handler, data);
dev->readtimer.expires = jiffies + msecs_to_jiffies(50);
add_timer(&dev->readtimer);
}
@@ -318,7 +317,7 @@ static void cadet_start_rds(struct cadet *dev)
{
dev->rdsstat = 1;
outb(0x80, dev->io);/* Select RDS fifo */
- setup_timer(&dev->readtimer, cadet_handler, (unsigned long)dev);
+ timer_setup(&dev->readtimer, cadet_handler, 0);
dev->readtimer.expires = jiffies + msecs_to_jiffies(50);
add_timer(&dev->readtimer);
}
diff --git a/drivers/media/radio/wl128x/fmdrv_common.c
b/drivers/media/radio/wl128x/fmdrv_common.c
index ab3428bf63fe..800d69c3f80b 100644
--- a/drivers/media/radio/wl128x/fmdrv_common.c
+++ b/drivers/media/radio/wl128x/fmdrv_common.c
@@ -543,13 +543,13 @@ static inline void fm_irq_common_cmd_resp_helper(struct
fmdev *fmdev, u8 stage)
* interrupt process. Therefore reset stage index to re-enable default
* interrupts. So that next interrupt will be processed as usual.
*/
-static void int_timeout_handler(unsigned long data)
+static void int_timeout_handler(struct timer_list *t)
{
struct fmdev *fmdev;
struct fm_irq *fmirq;
fmdbg("irq: timeout,trying to re-enable fm interrupts\n");
- fmdev = (struct fmdev *)data;
+ fmdev = from_timer(fmdev, t, irq_info.timer);
fmirq = &fmdev->irq_info;
fmirq->retry++;
@@ -1550,8 +1550,7 @@ int fmc_prepare(struct fmdev *fmdev)
atomic_set(&fmdev->tx_cnt, 1);
fmdev->resp_comp = NULL;
- setup_timer(&fmdev->irq_info.timer, &int_timeout_handler,
- (unsigned long)fmdev);
+ timer_setup(&fmdev->irq_info.timer, int_timeout_handler, 0);
/*TODO: add FM_STIC_EVENT later */
fmdev->irq_info.mask = FM_MAL_EVENT;
--
2.7.4
--
Kees Cook
Pixel Security
Re: [PATCH] media: pvrusb2: Convert timers to use timer_setup()
Eek, sorry, this uses timer_setup_on_stack() which is only in -next.
If you can Ack this, I can carry it in the timer tree.
Thanks!
-Kees
On Tue, Oct 24, 2017 at 5:22 PM, Kees Cook wrote:
> In preparation for unconditionally passing the struct timer_list pointer to
> all timer callbacks, switch to using the new timer_setup() and from_timer()
> to pass the timer pointer explicitly.
>
> Cc: Mike Isely
> Cc: Mauro Carvalho Chehab
> Cc: linux-media@vger.kernel.org
> Signed-off-by: Kees Cook
> ---
> drivers/media/usb/pvrusb2/pvrusb2-hdw.c | 64
> ++---
> 1 file changed, 36 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
> b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
> index ad5b25b89699..8289ee482f49 100644
> --- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
> +++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
> @@ -330,10 +330,10 @@ static void pvr2_hdw_state_log_state(struct pvr2_hdw *);
> static int pvr2_hdw_cmd_usbstream(struct pvr2_hdw *hdw,int runFl);
> static int pvr2_hdw_commit_setup(struct pvr2_hdw *hdw);
> static int pvr2_hdw_get_eeprom_addr(struct pvr2_hdw *hdw);
> -static void pvr2_hdw_quiescent_timeout(unsigned long);
> -static void pvr2_hdw_decoder_stabilization_timeout(unsigned long);
> -static void pvr2_hdw_encoder_wait_timeout(unsigned long);
> -static void pvr2_hdw_encoder_run_timeout(unsigned long);
> +static void pvr2_hdw_quiescent_timeout(struct timer_list *);
> +static void pvr2_hdw_decoder_stabilization_timeout(struct timer_list *);
> +static void pvr2_hdw_encoder_wait_timeout(struct timer_list *);
> +static void pvr2_hdw_encoder_run_timeout(struct timer_list *);
> static int pvr2_issue_simple_cmd(struct pvr2_hdw *,u32);
> static int pvr2_send_request_ex(struct pvr2_hdw *hdw,
> unsigned int timeout,int probe_fl,
> @@ -2373,18 +2373,15 @@ struct pvr2_hdw *pvr2_hdw_create(struct usb_interface
> *intf,
> }
> if (!hdw) goto fail;
>
> - setup_timer(&hdw->quiescent_timer, pvr2_hdw_quiescent_timeout,
> - (unsigned long)hdw);
> + timer_setup(&hdw->quiescent_timer, pvr2_hdw_quiescent_timeout, 0);
>
> - setup_timer(&hdw->decoder_stabilization_timer,
> - pvr2_hdw_decoder_stabilization_timeout,
> - (unsigned long)hdw);
> + timer_setup(&hdw->decoder_stabilization_timer,
> + pvr2_hdw_decoder_stabilization_timeout, 0);
>
> - setup_timer(&hdw->encoder_wait_timer, pvr2_hdw_encoder_wait_timeout,
> - (unsigned long)hdw);
> + timer_setup(&hdw->encoder_wait_timer, pvr2_hdw_encoder_wait_timeout,
> + 0);
>
> - setup_timer(&hdw->encoder_run_timer, pvr2_hdw_encoder_run_timeout,
> - (unsigned long)hdw);
> + timer_setup(&hdw->encoder_run_timer, pvr2_hdw_encoder_run_timeout, 0);
>
> hdw->master_state = PVR2_STATE_DEAD;
>
> @@ -3539,10 +3536,16 @@ static void pvr2_ctl_read_complete(struct urb *urb)
> complete(&hdw->ctl_done);
> }
>
> +struct hdw_timer {
> + struct timer_list timer;
> + struct pvr2_hdw *hdw;
> +};
>
> -static void pvr2_ctl_timeout(unsigned long data)
> +static void pvr2_ctl_timeout(struct timer_list *t)
> {
> - struct pvr2_hdw *hdw = (struct pvr2_hdw *)data;
> + struct hdw_timer *timer = from_timer(timer, t, timer);
> + struct pvr2_hdw *hdw = timer->hdw;
> +
> if (hdw->ctl_write_pend_flag || hdw->ctl_read_pend_flag) {
> hdw->ctl_timeout_flag = !0;
> if (hdw->ctl_write_pend_flag)
> @@ -3564,7 +3567,10 @@ static int pvr2_send_request_ex(struct pvr2_hdw *hdw,
> {
> unsigned int idx;
> int status = 0;
> - struct timer_list timer;
> + struct hdw_timer timer = {
> + .hdw = hdw,
> + };
> +
> if (!hdw->ctl_lock_held) {
> pvr2_trace(PVR2_TRACE_ERROR_LEGS,
>"Attempted to execute control transfer without
> lock!!");
> @@ -3621,8 +3627,8 @@ static int pvr2_send_request_ex(struct pvr2_hdw *hdw,
> hdw->ctl_timeout_flag = 0;
> hdw->ctl_write_pend_flag = 0;
> hdw->ctl_read_pend_flag = 0;
> - setup_timer(&timer, pvr2_ctl_timeout, (unsigned long)hdw);
> - timer.expires = jiffies + timeout;
> + timer_setup_on_stack(&timer.timer, pvr2_ctl_timeout, 0);
> + timer.timer.expires = jiffies + timeout;
>
> if (write_len && write_data) {
> hdw->cmd_de
Re: [PATCH] media: av7110: switch to useing timer_setup()
On Tue, Oct 24, 2017 at 5:40 PM, Dmitry Torokhov
wrote:
> In preparation for unconditionally passing the struct timer_list pointer to
> all timer callbacks, switch to using the new timer_setup() and from_timer()
> to pass the timer pointer explicitly.
>
> Also stop poking into input core internals and override its autorepeat
> timer function. I am not sure why we have such convoluted autorepeat
> handling in this driver instead of letting input core handle autorepeat,
> but this preserves current behavior of allowing controlling autorepeat
> delay and forcing autorepeat period to be whatever the hardware has.
>
> Signed-off-by: Dmitry Torokhov
Reviewed-by: Kees Cook
(with the Subject typo fixed)
Hans, since this depends on the input side not changing first, I think
it makes sense for Dmitry to carry this in the Input tree before the
Input timer updates. Would that be okay?
Thanks!
-Kees
> ---
>
> Note that this has not been tested on the hardware. But it should
> compile, so I have that going for me.
>
> drivers/media/pci/ttpci/av7110.h| 4 ++--
> drivers/media/pci/ttpci/av7110_ir.c | 40
> +
> 2 files changed, 20 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/media/pci/ttpci/av7110.h
> b/drivers/media/pci/ttpci/av7110.h
> index 347827925c14..0aa3c6f01853 100644
> --- a/drivers/media/pci/ttpci/av7110.h
> +++ b/drivers/media/pci/ttpci/av7110.h
> @@ -80,10 +80,11 @@ struct av7110;
>
> /* infrared remote control */
> struct infrared {
> - u16 key_map[256];
> + u16 key_map[256];
> struct input_dev*input_dev;
> charinput_phys[32];
> struct timer_list keyup_timer;
> + unsigned long keydown_time;
> struct tasklet_struct ir_tasklet;
> void(*ir_handler)(struct av7110 *av7110, u32
> ircom);
> u32 ir_command;
> @@ -93,7 +94,6 @@ struct infrared {
> u8 inversion;
> u16 last_key;
> u16 last_toggle;
> - u8 delay_timer_finished;
> };
>
>
> diff --git a/drivers/media/pci/ttpci/av7110_ir.c
> b/drivers/media/pci/ttpci/av7110_ir.c
> index ca05198de2c2..b602e64b3412 100644
> --- a/drivers/media/pci/ttpci/av7110_ir.c
> +++ b/drivers/media/pci/ttpci/av7110_ir.c
> @@ -84,9 +84,9 @@ static u16 default_key_map [256] = {
>
>
> /* key-up timer */
> -static void av7110_emit_keyup(unsigned long parm)
> +static void av7110_emit_keyup(struct timer_list *t)
> {
> - struct infrared *ir = (struct infrared *) parm;
> + struct infrared *ir = from_timer(ir, keyup_timer, t);
>
> if (!ir || !test_bit(ir->last_key, ir->input_dev->key))
> return;
> @@ -152,19 +152,20 @@ static void av7110_emit_key(unsigned long parm)
> return;
> }
>
> - if (timer_pending(&ir->keyup_timer)) {
> - del_timer(&ir->keyup_timer);
> + if (del_timer(&ir->keyup_timer)) {
> if (ir->last_key != keycode || toggle != ir->last_toggle) {
> - ir->delay_timer_finished = 0;
> + ir->keydown_time = jiffies;
> input_event(ir->input_dev, EV_KEY, ir->last_key, 0);
> input_event(ir->input_dev, EV_KEY, keycode, 1);
> input_sync(ir->input_dev);
> - } else if (ir->delay_timer_finished) {
> + } else if (time_after(jiffies, ir->keydown_time +
> + msecs_to_jiffies(
> + ir->input_dev->rep[REP_PERIOD]))) {
> input_event(ir->input_dev, EV_KEY, keycode, 2);
> input_sync(ir->input_dev);
> }
> } else {
> - ir->delay_timer_finished = 0;
> + ir->keydown_time = jiffies;
> input_event(ir->input_dev, EV_KEY, keycode, 1);
> input_sync(ir->input_dev);
> }
> @@ -172,9 +173,7 @@ static void av7110_emit_key(unsigned long parm)
> ir->last_key = keycode;
> ir->last_toggle = toggle;
>
> - ir->keyup_timer.expires = jiffies + UP_TIMEOUT;
> - add_timer(&ir->keyup_timer);
> -
> + mod_timer(&ir->keyup_timer, jiffies + UP_TIMEOUT);
> }
>
>
> @@ -184,12 +183,19 @@ static void input_register_keys(struct infrared *ir)
> int i;
>
> set_bit(EV_KEY,
Re: [PATCH v2] media: ttpci: remove autorepeat handling and use timer_setup
ut_dev);
> return err;
> }
> - input_dev->timer.function = input_repeat_key;
> - input_dev->timer.data = (unsigned long) &av7110->ir;
> +
> + /*
> +* Input core's default autorepeat is 33 cps with 250 msec
> +* delay, let's adjust to numbers more suitable for remote
> +* control.
> +*/
> + input_enable_softrepeat(input_dev, 250, 125);
>
> if (av_cnt == 1) {
> e = proc_create("av7110_ir", S_IWUSR, NULL,
> &av7110_ir_proc_fops);
> --
> 2.13.6
>
--
Kees Cook
Pixel Security
Re: [PATCH v2] media: ttpci: remove autorepeat handling and use timer_setup
On Fri, Nov 3, 2017 at 3:17 PM, Dmitry Torokhov wrote: > On Thu, Nov 02, 2017 at 10:16:58PM -0200, Mauro Carvalho Chehab wrote: >> Em Thu, 2 Nov 2017 16:50:37 -0700 >> Dmitry Torokhov escreveu: >> >> > On Thu, Nov 02, 2017 at 04:24:27PM -0700, Kees Cook wrote: >> > > On Tue, Oct 31, 2017 at 1:11 PM, Sean Young wrote: >> > > > Leave the autorepeat handling up to the input layer, and move >> > > > to the new timer API. >> > > > >> > > > Compile tested only. >> > > > >> > > > Signed-off-by: Sean Young >> > > >> > > Hi! Just checking up on this... the input timer conversion is blocked >> > > by getting this sorted out, so I'd love to have something either >> > > media, input, or timer tree can carry. :) >> > >> > Acked-by: Dmitry Torokhov >> > >> > From my POV the patch is good. Mauro, do you want me to take it through >> > my tree, or maybe you could create an immutable branch off 4.14-rc5 (or >> > 6) with this commit and I will pull it in and then can apply Kees input >> > core conversion patch? >> >> Feel free to apply it into your tree with my ack: >> >> Acked-by: Mauro Carvalho Chehab > > Applied and pulled Kees' patch to the input core (dropping the timer_data > business) on top. Awesome, thanks! :) -Kees -- Kees Cook Pixel Security
[PATCH] media: rc-core: use %s in rc_map_get() module load
rc_map_get() takes a single string literal for the module to load,
so make sure it cannot be used as a format string in the call to
request_module().
Signed-off-by: Kees Cook
---
On another security note, this raw request_module() call should have
some kind of prefix associated with it to make sure it can't be used to
load arbitrary modules. For example, request_module("probe-%s", name)
or something, as done for crypto, binfmts, etc.
---
drivers/media/rc/rc-main.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index 02e2f38c9c85..dca97aa0a51e 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -62,7 +62,7 @@ struct rc_map *rc_map_get(const char *name)
map = seek_rc_map(name);
#ifdef MODULE
if (!map) {
- int rc = request_module(name);
+ int rc = request_module("%s", name);
if (rc < 0) {
printk(KERN_ERR "Couldn't load IR keymap %s\n", name);
return NULL;
--
1.7.9.5
--
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] media: dib9000: avoid out of bound access
On Wed, Jun 18, 2014 at 3:02 PM, Heinrich Schuchardt wrote: > The current test to avoid out of bound access to mb[] is insufficient. > For len = 19 non-existent mb[10] will be accessed. > > A check in the for loop is insufficient to avoid out of bound access in > dib9000_mbx_send_attr. > > Signed-off-by: Heinrich Schuchardt > --- > drivers/media/dvb-frontends/dib9000.c | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/dvb-frontends/dib9000.c > b/drivers/media/dvb-frontends/dib9000.c > index e540cfb..6a71917 100644 > --- a/drivers/media/dvb-frontends/dib9000.c > +++ b/drivers/media/dvb-frontends/dib9000.c > @@ -1040,10 +1040,13 @@ static int dib9000_risc_apb_access_write(struct > dib9000_state *state, u32 addres > if (address >= 1024 || !state->platform.risc.fw_is_running) > return -EINVAL; > > + if (len > 18) > + return -EINVAL; > + > /* dprintk( "APB access thru wr fw %d %x", address, attribute); */ > > mb[0] = (unsigned short)address; > - for (i = 0; i < len && i < 20; i += 2) > + for (i = 0; i < len; i += 2) > mb[1 + (i / 2)] = (b[i] << 8 | b[i + 1]); Good catch on the mb[] access! However, I think there is still more to fix since b[i + 1] can read past the end of b: Say b is defined as "u8 b[3]". Passing len 3 means the second loop, with i==2 will access b[2] and b[3], the latter is out of range. -Kees > > dib9000_mbx_send_attr(state, OUT_MSG_BRIDGE_APB_W, mb, 1 + len / 2, > attribute); > -- > 2.0.0 > -- Kees Cook Chrome OS Security -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] media: dib9000: avoid out of bound access
On Wed, Jun 18, 2014 at 6:41 PM, Heinrich Schuchardt wrote:
> On 19.06.2014 01:50, Kees Cook wrote:
>>
>> On Wed, Jun 18, 2014 at 3:02 PM, Heinrich Schuchardt
>> wrote:
>>>
>>> The current test to avoid out of bound access to mb[] is insufficient.
>>> For len = 19 non-existent mb[10] will be accessed.
>>>
>>> A check in the for loop is insufficient to avoid out of bound access in
>>> dib9000_mbx_send_attr.
>>>
>>> Signed-off-by: Heinrich Schuchardt
>>> ---
>>> drivers/media/dvb-frontends/dib9000.c | 5 -
>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/media/dvb-frontends/dib9000.c
>>> b/drivers/media/dvb-frontends/dib9000.c
>>> index e540cfb..6a71917 100644
>>> --- a/drivers/media/dvb-frontends/dib9000.c
>>> +++ b/drivers/media/dvb-frontends/dib9000.c
>>> @@ -1040,10 +1040,13 @@ static int dib9000_risc_apb_access_write(struct
>>> dib9000_state *state, u32 addres
>>> if (address >= 1024 || !state->platform.risc.fw_is_running)
>>> return -EINVAL;
>>>
>>> + if (len > 18)
>>> + return -EINVAL;
>>> +
>>> /* dprintk( "APB access thru wr fw %d %x", address, attribute);
>>> */
>>>
>>> mb[0] = (unsigned short)address;
>>> - for (i = 0; i < len && i < 20; i += 2)
>>> + for (i = 0; i < len; i += 2)
>>> mb[1 + (i / 2)] = (b[i] << 8 | b[i + 1]);
>>
>>
>> Good catch on the mb[] access! However, I think there is still more to
>> fix since b[i + 1] can read past the end of b: Say b is defined as "u8
>> b[3]". Passing len 3 means the second loop, with i==2 will access b[2]
>> and b[3], the latter is out of range.
>
>
> b[] and len are provided by the caller of dib9000_risc_apb_access_write.
> dib9000_risc_apb_access_write cannot verify if the length of b[] matches len
> at all.
>
> Currently dib9000_risc_apb_access_write cannot handle odd values of len.
> This holds even true if b[] has been padded with zero to an even length: For
> odd values of len the last byte is not passed to dib9000_mbx_send_attr.
>
> What is left unclear is how odd values of len should be handled correctly:
>
> Should the caller provide a b[] padded with 0 to the next even number of
> bytes,
> or should dib9000_risc_apb_access_write take care not to read more then len
> bytes,
> or should odd values of len cause an error EINVAL.
>
> From what I read in the coding one source of the value of len is
> tuner_attach(), which is called from outside the dib9000 driver.
How about:
for (i = 0; i < len; i += 2) {
u16 val = b[i] << 8;
if (i + 1 < len)
val |= b[i + 1];
mb[1 + (i / 2)] = val;
That's defensive, and would have the same effect of callers doing the padding.
-Kees
>
> Heinrich
>
>
>>
>> -Kees
>>
>>>
>>> dib9000_mbx_send_attr(state, OUT_MSG_BRIDGE_APB_W, mb, 1 + len /
>>> 2, attribute);
>>> --
>>> 2.0.0
>>>
>>
>>
>>
>
--
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1 v2] media: dib9000: avoid out of bound access
On Thu, Jun 19, 2014 at 7:49 AM, Heinrich Schuchardt wrote: > This updated patch also fixes out of bound access to b[]. > > In dib9000_risc_apb_access_write() an out of bound access to mb[]. > > The current test to avoid out of bound access to mb[] is insufficient. > For len = 19 non-existent mb[10] will be accessed. > > For odd values of len b[] is accessed out of bounds. > > For large values of len an of bound access to mb[] may occur in > dib9000_mbx_send_attr. > > Signed-off-by: Heinrich Schuchardt > --- > drivers/media/dvb-frontends/dib9000.c | 13 + > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/dvb-frontends/dib9000.c > b/drivers/media/dvb-frontends/dib9000.c > index e540cfb..f75dec4 100644 > --- a/drivers/media/dvb-frontends/dib9000.c > +++ b/drivers/media/dvb-frontends/dib9000.c > @@ -1040,13 +1040,18 @@ static int dib9000_risc_apb_access_write(struct > dib9000_state *state, u32 addres > if (address >= 1024 || !state->platform.risc.fw_is_running) > return -EINVAL; > > + if (len > 18) > + return -EINVAL; > + > /* dprintk( "APB access thru wr fw %d %x", address, attribute); */ > > - mb[0] = (unsigned short)address; > - for (i = 0; i < len && i < 20; i += 2) > - mb[1 + (i / 2)] = (b[i] << 8 | b[i + 1]); > + mb[0] = (u16)address; > + for (i = 0; i + 1 < len; i += 2) > + mb[1 + i / 2] = b[i] << 8 | b[i + 1]; > + if (len & 1) > + mb[1 + len / 2] = b[len - 1] << 8; > > - dib9000_mbx_send_attr(state, OUT_MSG_BRIDGE_APB_W, mb, 1 + len / 2, > attribute); > + dib9000_mbx_send_attr(state, OUT_MSG_BRIDGE_APB_W, mb, (3 + len) / 2, > attribute); > return dib9000_mbx_get_message_attr(state, IN_MSG_END_BRIDGE_APB_RW, > mb, &s, attribute) == 1 ? 0 : -EINVAL; > } > > -- > 2.0.0 > That looks great, thanks! Reviewed-by: Kees Cook -Kees -- Kees Cook Chrome OS Security -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] [media] af9035: make sure loading modules is const
Make sure that loaded modules are const char strings so we don't
load arbitrary modules in the future, nor allow for format string
leaks in the module request call.
Signed-off-by: Kees Cook
---
drivers/media/usb/dvb-usb-v2/af9035.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/media/usb/dvb-usb-v2/af9035.c
b/drivers/media/usb/dvb-usb-v2/af9035.c
index 00758c83eec7..1896ab218b11 100644
--- a/drivers/media/usb/dvb-usb-v2/af9035.c
+++ b/drivers/media/usb/dvb-usb-v2/af9035.c
@@ -193,8 +193,8 @@ static int af9035_wr_reg_mask(struct dvb_usb_device *d, u32
reg, u8 val,
return af9035_wr_regs(d, reg, &val, 1);
}
-static int af9035_add_i2c_dev(struct dvb_usb_device *d, char *type, u8 addr,
- void *platform_data, struct i2c_adapter *adapter)
+static int af9035_add_i2c_dev(struct dvb_usb_device *d, const char *type,
+ u8 addr, void *platform_data, struct i2c_adapter *adapter)
{
int ret, num;
struct state *state = d_to_priv(d);
@@ -221,7 +221,7 @@ static int af9035_add_i2c_dev(struct dvb_usb_device *d,
char *type, u8 addr,
goto err;
}
- request_module(board_info.type);
+ request_module("%s", board_info.type);
/* register I2C device */
client = i2c_new_device(adapter, &board_info);
--
1.9.1
--
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] [media] anysee: make sure loading modules is const
Make sure that loaded modules are const char strings so we don't
load arbitrary modules in the future, nor allow for format string
leaks in the module request call.
Signed-off-by: Kees Cook
---
drivers/media/usb/dvb-usb-v2/anysee.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/media/usb/dvb-usb-v2/anysee.c
b/drivers/media/usb/dvb-usb-v2/anysee.c
index d3c5f230e97a..ae917c042a52 100644
--- a/drivers/media/usb/dvb-usb-v2/anysee.c
+++ b/drivers/media/usb/dvb-usb-v2/anysee.c
@@ -630,8 +630,8 @@ error:
return ret;
}
-static int anysee_add_i2c_dev(struct dvb_usb_device *d, char *type, u8 addr,
- void *platform_data)
+static int anysee_add_i2c_dev(struct dvb_usb_device *d, const char *type,
+ u8 addr, void *platform_data)
{
int ret, num;
struct anysee_state *state = d_to_priv(d);
@@ -659,7 +659,7 @@ static int anysee_add_i2c_dev(struct dvb_usb_device *d,
char *type, u8 addr,
goto err;
}
- request_module(board_info.type);
+ request_module("%s", board_info.type);
/* register I2C device */
client = i2c_new_device(adapter, &board_info);
--
1.9.1
--
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
